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.

Sign up for GitHub

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

Jump to bottom

Adding Astronomical units #679

Closed
ebfortin opened this issue Jul 17, 2019 · 15 comments
Closed

Adding Astronomical units #679

ebfortin opened this issue Jul 17, 2019 · 15 comments
Labels
enhancement

Comments

@ebfortin
Copy link
Contributor

ebfortin commented Jul 17, 2019 •
edited
Loading

Is your feature request related to a problem? Please describe.
I'm working on a planet generation software and am using astronomical units all over. Right now these units are treated all as double. Which is exactly what UnistNet is there to resolve.

Describe the solution you'd like
Astronomical units, like AU and mass ratio to solar mass, could be added to the relevant unit class like Length.

Describe alternatives you've considered
Adding custom units. However this, as indicated, is not that well designed right now and I don't want to lose link with UnitsNet future releases. Also it makes sense to support these units used in a very large field of study.
@angularsen
Copy link
Owner

Hi and thanks for describing the feature. I think this sounds all very interesting and definitely something that belongs in the library. Would you be interested in doing one or more pull requests? I'm happy to assist.

https://github.com/angularsen/UnitsNet#want-to-contribute

@ebfortin
Copy link
Contributor Author

For sure I would be interested. I will follow the link and prepare the code. I've checked how it is designed and adding what I want to the json files seem straightforward enough.

@angularsen
Copy link
Owner

Perfect! Look forward to it. Don't hesitate to ask if something is not clear.

@ebfortin
Copy link
Contributor Author

Why is Power using decimal has its base type instead of double for almost all other units I've looked at?

@angularsen
Copy link
Owner

angularsen commented Jul 18, 2019 •
edited
Loading

It's because it has a very wide range of units from very small to very large, so the conversions would get very poor precision because they have to convert via the base unit.
It's fixable, but for now there are around 3 quantities that use decimal internally as a workaround.

@angularsen
Copy link
Owner

Some discussions on improving precision:

@ebfortin
Copy link
Contributor Author

ebfortin commented Jul 18, 2019 via email

@ebfortin
Copy link
Contributor Author

ebfortin commented Jul 18, 2019 via email

@ebfortin
Copy link
Contributor Author

ebfortin commented Jul 18, 2019 via email

@angularsen
Copy link
Owner

Can you create a pull request so I can see what code changes you have? I don't see immediately what is wrong from your description.

Tolerance can be overridden by overriding the tolerance property values in the LengthTestBase class (or something like that), and setting it to something different than 1e-5 that I believe is the default.

@ebfortin
Copy link
Contributor Author

ebfortin commented Jul 18, 2019 via email

@angularsen
Copy link
Owner

angularsen commented Jul 18, 2019 •
edited
Loading

Sorry, I miswrote, you should edit LengthTests and override like this:

        protected override double YourUnitTolerance => 1e-3;

This property is autogenerated in LengthTestsBase.

Update: Forgot to specify the tolerance value.

@angularsen
Copy link
Owner

Update: Forgot to specify the tolerance value.

@ebfortin
Copy link
Contributor Author

ebfortin commented Jul 18, 2019 via email

@angularsen
Copy link
Owner

Fixed in #680

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@angularsen @ebfortin