Replace unyt.amu
with Unit("amu")
to perserve mass values from mBuild
#759
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a quick PR that changes how we use amu units from Unyt. There is some inconsistent behavior with how Unyt handles amu specifically compared to other units. See the issue here
In
from_mbuild
I believe the goal is to copy over the particle mass as is, but to add on amu units since mBuild uses amu for mass. But, when we do this usingu.amu
we end up converting the mass to kg. This happens becauseu.amu
is already aunyt_quantity
that is in kg. In the issue, there is a suggestion to useUnit('amu')
instead.Example: You can see
u.amu
does not have the same behavior asu.g
oru.g/u.mol
, butUnit("amu")
does.Here is what the change would look like in gmso. IMO, keeping the mass values the same as they are in mBuild, and actually using units of amu instead of kg is preferable.
Code snippet:
Using
unyt.amu
(current behavior):Using
Unit("amu")
: