-
Notifications
You must be signed in to change notification settings - Fork 230
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
Update global uncertainty module to work with MUQ 2 #1738
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1738 +/- ##
=========================================
- Coverage 44.2% 44.2% -0.01%
=========================================
Files 83 83
Lines 21533 21529 -4
Branches 5645 5644 -1
=========================================
- Hits 9519 9517 -2
+ Misses 10961 10945 -16
- Partials 1053 1067 +14
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks mostly fine to me. Doesn't the documentation need updated as a result of the uncertainty option changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of minor things, but the notebook appears to work as promised, so I think this is mostly good to go. I also think that the documentation might need updating. In the future, we should also add some unit tests for the global uncertainty module
580a9df
to
924a1c4
Compare
Everything except the extraneous character in the docstring looks good. Rebase and I'll merge |
Also rename module from muq to globaluncertainty
It is no longer used by any notebooks
Rebased. |
Thanks for the PR @mliu49 ! |
Motivation or Problem
The global uncertainty module of RMG previously used MUQ 1, relying on a semi-functional conda binary which only worked on Linux. MUQ 1 has since been deprecated in favor of MUQ 2.
Instead of updating the MUQ 1 binary to work with Python 3, I instead chose to build a new MUQ 2 binary from scratch.
Description of Changes
This PR implements Python 3, RMG py3, and MUQ 2 transition changes in the
rmgpy.tools.globaluncertainty
module (renamed fromrmgpy.tools.muq
).This also updates the related IPython notebook.
Note: This is currently based on #1735.
Testing
I have run a couple of examples and the IPython notebook, so everything appears to be working.
I have not yet done any benchmarks to see if the results are the same as before.
To Do