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

Make yt.units a shim that wraps unyt #1785

Closed
ngoldbaum opened this issue May 16, 2018 · 8 comments
Closed

Make yt.units a shim that wraps unyt #1785

ngoldbaum opened this issue May 16, 2018 · 8 comments
Labels
Milestone

Comments

@ngoldbaum
Copy link
Member

ngoldbaum commented May 16, 2018

This issue tracks the task of replacing yt.units with unyt: https://github.com/yt-project/unyt

There are some differences in unyt with respect to yt.units so we will need to be careful to make sure that everything works the same after integrating. We also need to check the codebase to see if there are places where we can clean things up. These differences are summarized below:

  • unyt allows you to do e.g. data_with_units + data_without_units*data_with_units.units, which obviates the need to use unit_quantity or uq.
  • SI <-> MKS Electromagnetic conversions do not need to use an equivalence
  • Results of operations that have complex units that have dimensions equal to a known unit name in the unit system being used are reduced to an equivalent unit in that unit system. For example, if something comes out of an operation with units like erg * cm/km, by default the result will be presented in erg (e.g. the cm/km will be converted to a constant that gets multiplied by the value of the object).
  • MKS is the default unit system
  • If you do unyt_array([1, 2, 3], 'code_length') you don't get a yt-specific error message suggesting to use ds.arr like if you do YTArray([1, 2, 3], 'code_length').
@chummels
Copy link
Member

Consistent with our discussion two weeks ago on a PR in unyt: yt-project/unyt#2 , Will you make it so the astrophysics context stuff defaults to cgs? I think you're going to find a lot of the current users upset if all of a sudden their projections are in kg/m^2 and so forth.

@ngoldbaum
Copy link
Member Author

Yes, that's why I included "MKS is the default unit system" in the bullet points.

@chummels
Copy link
Member

I saw that, but I guess we're talking past each other. So your bullet points list the changes in unyt that we need to effectively cover up so that the yt experience is the same for users?

@jzuhone
Copy link
Contributor

jzuhone commented May 16, 2018

@chummels my view is that the experience from the point of view of yt is identical--the current default unit system in yt is cgs, so it should be that way when we use unyt as a backend.

@jzuhone
Copy link
Contributor

jzuhone commented May 16, 2018

Though I should say that I consider the first three bullet points enhancements and yt 4.0 should have those changes.

@ngoldbaum
Copy link
Member Author

Yup, exactly, although with the qualification that data from fields that we don't support yet might load data with MKS or other unit systems that are commonly used in that field. The astrophysics domain context will definitely have CGS units by default.

@chummels
Copy link
Member

OK cool, then we're all on the same page. Sorry for my mis-reading of your description.

@munkm
Copy link
Member

munkm commented Apr 3, 2020

I think this is is satisfied now, so I'm going to close it. Feel free to reopen if you all think that yt.units still hasn't outsourced enough to unyt.

@munkm munkm closed this as completed Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants