Updates species dictionary path for RMG #828
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the species dictionary filename from "species_dictionary.txt" to "dictionary.txt" in the save_thermo_lib function to align with RMG 3.9+ conventions.
Changes:
- Modified
arc/plotter.pyto change the species dictionary filename for thermo libraries from "species_dictionary.txt" to "dictionary.txt" - Added a comment explaining the naming convention difference between kinetics and thermo libraries
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
arc/plotter.py
Outdated
| # RMG kinetics libraries expect dictionary.txt (thermo uses species_dictionary.txt). | ||
| species_dict_path = os.path.join(lib_path, 'dictionary.txt') |
There was a problem hiding this comment.
There is a critical discrepancy between the PR description and the code changes. The PR description states "Updates the species dictionary path to 'dictionary.txt' to align with the expected file name for RMG kinetics libraries, whereas thermo uses 'species_dictionary.txt'." This clearly states that kinetics libraries expect dictionary.txt and thermo uses species_dictionary.txt. However, this change is being made in the save_thermo_lib function (line 770 docstring confirms it "Save an RMG thermo library"), not the save_kinetics_lib function. The code change contradicts the PR description. Either the PR description is incorrect, or the change is being applied to the wrong function. Based on the documentation in docs/source/output.rst which shows kinetics libraries having dictionary.txt, it appears that save_kinetics_lib (line 909) should be updated instead of or in addition to this function.
calvinp0
force-pushed
the
rmg_kin_dict
branch
5 times, most recently
from
5d5a2fa to
a033397
Compare
alongd
left a comment
alongd
left a comment
There was a problem hiding this comment.
Thanks, I added two minor comments
arc/plotter.py
Outdated
| with open(thermo_file, 'w') as f: | ||
| f.write(thermo_txt) | ||
|
|
||
| # Thermo libraries use species_dictionary.txt. |
There was a problem hiding this comment.
please remove (or change?) the comment, thermo libraries don't use an external dictionary file. Looks like here we store the dictionary file for the user, for post-processing the ARC output. let's be consistent with the RMG naming and change the filename below to dictionary.txt
arc/plotter.py
Outdated
| f.write(reactions_txt) | ||
|
|
||
| species_dict_path = os.path.join(lib_path, 'species_dictionary.txt') | ||
| # RMG kinetics libraries expect dictionary.txt. |
calvinp0
force-pushed
the
rmg_kin_dict
branch
from
a033397 to
67ff4c9
Compare
Updates the species dictionary path to "dictionary.txt" to align with the expected file name for RMG kinetics libraries, whereas thermo uses "species_dictionary.txt". It appears that RMG, since 3.9, prefers it to be called dictionary.txt Removes redundant comments Removes comments that state the expected file name for species dictionaries in kinetics libraries. This information is either redundant or outdated.
Includes setuptools as a dependency in the environment file. This ensures that the package installation process has access to required tools and resources.
2 participants
Updates the species dictionary path to "dictionary.txt" to align with the expected file name for RMG kinetics libraries, whereas thermo uses "species_dictionary.txt".
It appears that RMG, since 3.9, prefers it to be called dictionary.txt