SMILES to PolymerizeIt! ready files#36
Conversation
|
Thanks @tramp037 for getting started with integrating PolymerizeIt into MuPT. We can first work internally on incorporating the feedback provided in yesterday's meeting and address the failing tests before making it ready for review. Failing testsThe first test fails with the following error: I am unclear on why this particular notebook is a part of the CI workflow. However, it looks like it has something to do with polymer generation from SMILES (MuPT-hub/mupt-examples#20). It may have some necessary checks related to the following MuPT functions that we are importing in https://github.com/tramp037/mupt/blob/polymerizeit/mupt/reactions/polymerizeit/react_pi.py: If so, it would be worthwhile to look at it, but I am not sure where to access it from. It is not available in the Maybe @timbernat can guide us further on this. Do we really need to pass this test to integrate our functionality into MuPT? MuPT integrationWe are probably looking at something like make_pi to be the front-facing module for running PolymerizeIt via MuPT. In that case, would it make sense to provide the variable values via a config file? That may help users better track multiple runs with minor tweaks in variables, though it would be fine to keep both options open. In any case, we need to think about what the minimum possible inputs would be to run a MuPT workflow. We can probably have only "science-related" variables declared here (like monomer SMILES strings and processing conditions like T, P, density, etc.). Then, variables required by the PolymerizeIt! config file (currently called the inputs file)—like reaction criteria, cross-linking criteria, charge update criteria, etc.—could be passed to a separate config file internally. We will have a default flowchart like you have currently, but the MuPT module help string should contain enough details to point users to the PolymerizeIt config file variables if they need to tweak the protocol flowchart. |
That's my bad, I renamed an example as part of examples #20 you mentioned but forgot to update the notebook runner call. I just updated that now and verified CI on main passes; give the CI another run on the fork and see if that allows the tests to complete.
The test does need to pass yes, but it's not your responsibility to get it working here; I broke it, and I'll fix it
Because we also want to ensure that code updates don't break examples, or that examples don't break as dependencies update. These are called after the main code tests are run (here, if you're curious). We may consider having the notebook tests be a separate CI run, though that of course means each CI run has to individually install a mupt-capable environment, which comes with extra overhead. There are tradeoffs for either approach
That would make the most sense, and is what I'm starting to move towards in #20 mentioned earlier. We want be as explicit as we can and hard-code as little as possible at this phase, even if it means the code is a little more verbose/requires more setup. Eventually I'll be stripping out and generalizing the reaction mapping components, and the PI/REACTER subpackages will mostly handle the specifics of generating valid inputs given a config (separate logic from parameters) |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Description
This pull request provides functionality for using SMILES and the MuPT representation to generate PolymerizeIt! ready files to perform reactions and generate polymer configurations.
Todos
Notable points that this PR has either accomplished or will accomplish.
Questions
Status