Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement std::iter::Sum #300

Closed
jacg opened this issue Apr 7, 2022 · 6 comments
Closed

Implement std::iter::Sum #300

jacg opened this issue Apr 7, 2022 · 6 comments

Comments

@jacg
Copy link
Contributor

jacg commented Apr 7, 2022

The orphan rule is really getting me down. At every step I come across third-party traits that I would like to have implemented for uom quantities, and there's nothing much that can be done. Wrapping uom types in newtypes seems too expensive. Any words of wisdom?

In the case of std::iter::Sum, I would argue that it should be implemented in uom itself. I'd be happy to contribute a PR if you agree.

If so, do you have any hints, pointers, warnings?

[Another trait that I'll be missing sorely fairly soon, is hdf5::H5Type but the case for including it in uom is not as strong.]

@jacg
Copy link
Contributor Author

jacg commented Apr 9, 2022

It turns out that it already is implemented.

I got confused because I failed to spot the lone & in the deluge of noise in the error message.

On the tangential issue of the orphan rule in general: using some of the macros that uom provides to create systems of units, should create them in the local crate, and thus avoid the orphan rule, should it not?

From the documentation, it's not clear to me whether ISQ! merely creates aliases to types defined in the uom crate, or whether it defines new types in the crate where it is used. If new base units are used, than I guess it must create new types in the local crate.

@jacg jacg closed this as completed Apr 9, 2022
@iliekturtles
Copy link
Owner

I just realized that I had a reply typed up and never posted it a couple days ago! Was the & issue implementations of V vs. &V?

I had a bunch of comments about the orphan rule and will see if I can type them up again later this weekend.

@jacg
Copy link
Contributor Author

jacg commented Apr 9, 2022

Was the & issue implementations of V vs. &V?

It was.

Essentially I did .into_iter().sum() on a &[SomeQuantity].

Chucking in a .cloned() made the problem go away.

I had a bunch of comments about the orphan rule and will see if I can type them up again later this weekend.

Thanks!

@iliekturtles
Copy link
Owner

#38 will eventually remove the need to .clone().

I've attempted to reproduce my thoughts on the orphan rule and your specific situation below. Probably not as comprehensive as the initial post I wrote and somehow lost.

  • As you mentioned you could add another level of newtypes. Do the conversion as you write to or read from hdf5 data. This can be done today but is a lot of busy work and is another point for errors.
  • Use one less level of newtypes by extracting the underlying storage type from the uom type at the hdf5 boundary. Also able to be done today, but the same busy work and possibly more error prone than the previous point.
  • Just use features and implement the traits in uom. Hesitance about this solution discussed in Testing for approximate equality of floats. #288.
  • After v1.0 one of the things I want to explore is how how the raw data (quantities, units, conversion factors) gets converted into Rust code. I've started experimenting with proc macros in the dev-macros branch. I've also thought about using a build.rs or a separate tool and generating the code prior to the compile process. Related to this I've also thought about how the types are generated. e.g. Should SI types (length, mass, ...) be generated in the end-users crate? Orphan problem solved! ... but then you introduce the new problem where two crates using uom would each have their own types.

As an aside from this issue, I can't put as much time into uom as I would like. You've been submitting a bunch of issues and questions which is great! Do you have a priority list of what is most important so I can focus my limited time?

@jacg
Copy link
Contributor Author

jacg commented Apr 10, 2022

Use one less level of newtypes by extracting the underlying storage type from the uom type at the hdf5 boundary.

That's what I've done so far. But, as you say, it involves busy work and leaves scope for errors.

Whilst we're on the topic, here's another trait where I'm having to do something similar: https://docs.rs/nalgebra/latest/nalgebra/trait.ComplexField.html. I suspect that this one is of more general interest than hdf5.

I can't put as much time into uom as I would like. You've been submitting a bunch of issues [...]

I really appreciate it when you reply promptly, but please don't feel under any pressure to do so.

Do you have a priority list of what is most important so I can focus my limited time?

I think you should prioritize what you find most interesting and satisfying to work on, or what would most benefit the project as a whole in your opinion, rather than catering for my needs, or my perception of priorities, which changes relatively quickly depending on what problem I happen to be trying to solve at any given time.

If anything, maybe provide some high-level documentation of the structure of the project, and some of the more important design choices or principles. This might enable me (and others) to answer more of my own questions, or find solutions more quickly, or even be able to submit more interesting PRs for more involved issues.

In terms of coding, I can hack my way around most of the issues I've raised, with more or less ugly/cumbersome/error-prone workarounds. But the lack of a clean way to define or refer to ad-hoc derived units (such as reciprocal-length) is probably the most irksome. If we could get something like dimensioned's derived macro or even its ability to use operators to combine units (or quantities in the case of uom), that would be great.

@iliekturtles
Copy link
Owner

Some initial discussion about nalgebra in #237.

I'm going to work through some of the open PRs then I'll see about taking a look at improving ergonomics for ad-hoc quantities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants