Skip to content

Conversation

@AntoniaBerger
Copy link

This pull request enhances the src/verify_cadet-core.py script by adding functionality to test reaction models and integrating them into the existing workflow. The most important changes include importing a new module, enabling reaction tests, and implementing specific reaction test cases.

Additions to reaction testing:

  • Added the multiple_reactions module import to support reaction testing functionality. ([src/verify_cadet-core.pyR38](https://github.com/cadet/CADET-Verification/pull/57/files#diff-32a96d33bd56851c4b04cc2065f66a8a1fa3ced0921e0f5a4d6d88cb160c44f3R38))
  • Introduced a new configuration flag, run_reaction_tests, set to True by default, to enable or disable reaction tests. ([src/verify_cadet-core.pyR60](https://github.com/cadet/CADET-Verification/pull/57/files#diff-32a96d33bd56851c4b04cc2065f66a8a1fa3ced0921e0f5a4d6d88cb160c44f3R60))
  • Implemented three new reaction test cases (multiple_reactions_grm_test_bulk, multiple_reactions_lrmp_test_bulk, and multiple_reactions_mct_test_bulk) and integrated them into the workflow. These tests generate output files and optionally delete them after execution. ([src/verify_cadet-core.pyR158-R168](https://github.com/cadet/CADET-Verification/pull/57/files#diff-32a96d33bd56851c4b04cc2065f66a8a1fa3ced0921e0f5a4d6d88cb160c44f3R158-R168))

Copy link
Contributor

@jbreue16 jbreue16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for starting this, great to finally have some reaction tests here!
I think once you have verified on your end that the results match for the old and new interface, we should only implement the new interface, otherwise we'll have to remove that code again in the future when the old interface is deprecated. I suggest you compute reference solutions with the new interface, store them under data/CADET-Core_reference/reactions, and update the test functions to compare to that reference solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants