-
Notifications
You must be signed in to change notification settings - Fork 141
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
Exporting to Java syntax messes up uncertainty units #21
Comments
This particular issue is fixed by ReactionMechanismGenerator/RMG-Py@fed3b59 but I have opened ReactionMechanismGenerator/RMG-Py#144 to remind us to look for other occurrences of uncertainty units being wrong. |
pierrelb
pushed a commit
to pierrelb/RMG-Py
that referenced
this issue
Nov 1, 2013
In commit 851b2ba it was changed so that Quantitiy.value and Quantity.uncertainty were now in customizable units, and Quantitiy.value_si was the SI value. However, there was no uncertainty_si. Now the uncertainty is also stored in SI units, like the value, and converted to output units on demand. This change should be transparent, i.e. not change anything that was correct before, but it does create a new Quantity.uncertainty_si property, which is what we should have been using instead of .uncertainty in various places. i.e. we now need to go around using this to fix other bugs, such as @GreenGroup/RMG-databaseReactionMechanismGenerator#21 ReactionMechanismGenerator/RMG-database#21 Also to do: do the same thing for the ArrayQuantity class.
pierrelb
pushed a commit
to pierrelb/RMG-Py
that referenced
this issue
Nov 1, 2013
…tabase rules. This should close @GreenGroup/RMG-databaseReactionMechanismGenerator#21 ReactionMechanismGenerator/RMG-database#21 But there are probably other places the same bug exists (i.e. foo.uncertainty should be replaced with foo.uncertainty_si) Also, why don't any of the Quantity unit tests check uncertainties at all? Hmm...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Import this:
and you get this
which becomes this
when you export it again.
The uncertainty on A is six orders of magnitude larger (m3/cm3), and on Ea is 4184 times smaller (J/kCal).
The text was updated successfully, but these errors were encountered: