Update Disproportionation family in the test database #2780
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dsiproportionation was converted to an autogenerated SIDT tree years ago, but we're still using a highly outdated version for testing. I'm only updating this one because doing so will allow us to make tests for the new format, and for now that is enough.
In the test_degeneracy_same_reactant_different_resonance_structure() test, I got rid of the assertion checking that the reaction matches a particular node in the tree because this will change any time the tree is regenerated, and the test is about degeneracy with different resonance structures, which already has an assertion.
|
I understand why nobody ever updates this. You'd also have to regenerate a lot of the example chemkin files (see RMG-Py/test/rmgpy/test_data/parsing_data/chem_annotated.inp) and I don't know how you get RMG to run using the test database instead of the regular database. Probably people have some manual workaround. This update is far more complicated than the change that motivated this PR, so I'm tabling this for now. I might have to come back and update the test database when it comes time to implement tests for the uncertainty module as a whole, but for now I just need tests for the extract_source_from_comment() function, and I can do that with the current version of the database. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation or Problem
The Disproportionation family was converted to an autogenerated SIDT tree years ago, but we're still using a highly outdated version for testing because there's no automatic transfer from RMG-database, and it takes effort to update since a lot of unit tests were written for that version.
The fact that this version of the database is so old should be reason enough to change it: what's the point of testing RMG-Py using entries that no longer resemble RMG-database? But I also need a decent sized autogenerated kinetics tree for some of the tests I'll be writing for uncertainty estimation, and Disproportionation has been my go-to example. So, that's why I'm updating this family in the test database.
Description of Changes
I copied the latest Disproportionation family from RMG-database cbc8f9f5a11c179208ad078d4010afc8355698fa and put it inside RMG-Py's test database. Now it's just a matter of updating all the failing unit tests so they know to expect a newer version of this kinetics tree:
Some of the highlights so far are
Testing
This change only affects the RMG-Py tests, and so it should be enough to make sure the tests are still passing.