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

Generic Parameters For Measurement Quantities #666

Open
ZacharyPatten opened this issue May 8, 2019 · 16 comments
Open

Generic Parameters For Measurement Quantities #666

ZacharyPatten opened this issue May 8, 2019 · 16 comments
Labels
enhancement pinned Issues that should not be auto-closed due to inactivity.
Milestone

Comments

@ZacharyPatten
Copy link

Generic Types

Right now, UnitsNet can only support "double" types. If you use generic types on the quantities you can allow for support of other types than just double.

I have working prototypes for Length, Mass, and Angle measurement types using generic paramters here:
https://github.com/ZacharyPatten/Towel

I wanted to share my patterns with UnitsNet, because they could help improve UnitsNet should they be adopted. However, there are pros/cons to my pattern, and it would be a massive overhaul of UnitsNet to incorporate generics, so it might not be feasible for the project to adopt the patterns.

Performance Optimization

The conversions in UnitsNet require a double conversion to/from a base unit. In that Towel project, I am using multiplication tables (in the form of jagged arrays) to cache the conversions between units. The index of the multiplication tables are provided by the values on the unit enums. This allows Towel to perform unit conversions with a single operation rather than double as UnitsNet is currently doing.

@angularsen
Copy link
Owner

angularsen commented May 8, 2019

Thanks for reporting @ZacharyPatten! As you described in the email, you are not interested in pursuing these improvements in UnitsNet yourself, just pointing out some source of reference we can look at when we address these two points.

There are a number of related issues on this already:

Below is my reply in the email for future reference.

To address your points:

  1. Yup, this is a pain point for some users. We are currently restricted by using struct instead of class, which is by design in order to get value vs reference semantics (no null checking, pass by value, more similar to .NET types like DateTime and TimeSpan). We do however have some open discussions on this topic already and although I have probably been the biggest opponent to changing the semantics without carefully evaluating the pros/cons first, I do see that there are considerable wins by moving to class; generics, inheritance, a lot less duplicated code and smaller binary size.

  2. Performance has not been a priority in the past, but I am always open to ideas. We don't do any caching so that sounds fairly straight forward to add. We also recently refactored the conversions from hard coded switches (in generated code) to dynamic lookup tables. It adds another method invocation in the conversion, not sure about that perf impact, but it gives us a constant lookup time and the possibility of adding direct conversion functions from say meter to centimeter - and maybe even automating conversions between prefixes (centi- to kilo-). It also opens up the option for plugging in third party conversions by the consumers. I guess we'll just have to profile it and see how well it performs, but again, not a big priority for now.

So yeah, absolutely valid points, but I don't have the time or enough personal interest in these particular changes to perform them myself anytime soon, but if you or someone else wants to take it on I'm happy to assist on any pull requests!

Thanks for the project link, it will be useful to see how you tackled similar things if we pursue generics and performance later.

@ZacharyPatten
Copy link
Author

ZacharyPatten commented May 8, 2019

In regards to #285 I see that MathNet was mentioned as an example of generic mathematics. Do not copy MathNet's pattern. It is not scaleable. You shouldn't use abstract methods or other forms of inheritance. You should use runtime compilation.

My Towel project is using runtime compilation in order to perform generic mathematics. This is a much better pattern because you don't have to write a custom implementation for every type you want to support.

I wrote an old blog post about this topic if anyone is interested. However I wrote the blog post before I knew about Linq expressions!!!! Linq expressions (as I'm using in the Towel project) allow for very clean runtime compilation code. But you may still find the blog post interesting:
http://towelcode.com/old-c-generic-math-article-12-may-2015/

Hope that helps.

@angularsen
Copy link
Owner

Thanks for your insights, I just read the blog post now and you have definitely ventured into some terrain that I haven't before. Interesting stuff about compiling at runtime.

We have used c# code generation (pre-compile, not at runtime) extensively in this project and that would be one option for us as well, but it does bloat the binary size quite a bit so if we can instead reuse code for the the 3 main numeric types (float, double, decimal) that will probably save us a bunch.

I believe we did discover some options for arithmetic with generics at some point - I just don't remember the details from the top of my head. @tmilnthorp had some ideas I believe.

Will definitely check out your linq expressions and runtime compilation for inspiration 👍

@ZacharyPatten
Copy link
Author

ZacharyPatten commented May 30, 2019

Hey. I just wanted to mention that I got prototypes of "Speed" and "Acceleration" working with generics working in Towel if you want to take a look. This is significant because those are not base measurement types (they are complex/derived measurements).

It required a different pattern than UnitsNet currently uses. UnitsNet only has one enum value per quantity, but the prototypes in Towel use potentially multiple enum values per quantity/measurement.

Towel Speed Construction Example:

Speed<double> a = new Speed<double>(1.5d, Meters / Seconds);
Speed<double> b = new Speed<double>(2.5d, Knots); // converted to length & time units under the hood

Towel Acceleration Construction Example:

Acceleration<double> c = new Acceleration<double>(3.5d, Meters / Seconds / Seconds);

The reason I found this was necessary is because if you want to support non-rational types (example: int), then you always need to auto-convert from larger unit to smaller unit so there is minimal rounding (won't round to zero for int).

I did some very primitive speed testing and my prototypes appear to still be faster (for speed and acceleration) than UnitsNet currently is regardless of the extra enum allocations.

Still a lot of testing/tinkering to do, but it is working so far. :)

If you don't care about supporting integer types, you probably don't need to store multiple enum values per quantity/measurement, but I wanted to support "int" as much as possible in Towel.

Hope that makes sense. Just trying to help.

@angularsen
Copy link
Owner

Thanks a lot for chiming in @ZacharyPatten , I'm super busy with work these days and have a hard time following up, but it is much appreciated that you drop knowledge and experiences for us to learn from! I don't think we will ever support the "int" scenario you mention, but interesting to learn how you approached it. Multiple enum values is interesting, I'd have to think some more on it to comment anything meaningful on it though :-)

@stale
Copy link

stale bot commented Sep 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 24, 2019
@angularsen
Copy link
Owner

Pinning this issue, at some point we need to address this.

@angularsen angularsen added pinned Issues that should not be auto-closed due to inactivity. and removed wontfix labels Sep 24, 2019
@tmilnthorp
Copy link
Collaborator

Would you be amenable to a major release soon with only this?

@angularsen
Copy link
Owner

Absolutely, if we can get generics working so we can get rid of the mix of double + decimal as well as allow the consumer to choose numeric type (float, double, decimal) then I'm game for a breaking change.

Note also that #651 might be getting some traction too, which would be perfect to include in a major bump.

@angularsen
Copy link
Owner

I don't recall, does this mean we have to go to class or can we still keep struct?

@ZacharyPatten
Copy link
Author

The generics will work with either classes or structs. That is a seperate topic of which is better classes or structs? I was using "struct" in my code, but I have yet to do some performance testing.

@tmilnthorp
Copy link
Collaborator

I was going to leave it as struct

@angularsen
Copy link
Owner

Awesome, then I don't see any reason not to go forward with this.

@angularsen
Copy link
Owner

We can't merge this into master branch until all breaking changes are ready, so I just created release/v5 branch for this purpose. Point any breaking change PRs there.

@WhiteBlackGoose
Copy link

A concept of a very generic units of measure library that may help you somehow: github.

@angularsen
Copy link
Owner

Interesting use of the new Math API @WhiteBlackGoose 👍 We have been contemplating how we can best utilize it for UnitsNet. I will keep this reference in mind!

@angularsen angularsen modified the milestones: 5.0, vNext Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pinned Issues that should not be auto-closed due to inactivity.
Projects
None yet
Development

No branches or pull requests

4 participants