Auto optical isomer ex symmetry arkane #1571
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1571 +/- ##
=======================================
Coverage 41.67% 41.67%
=======================================
Files 166 166
Lines 28452 28452
Branches 5855 5855
=======================================
Hits 11858 11858
Misses 15773 15773
Partials 821 821Continue to review full report at Codecov.
|
goldmanm
force-pushed
the
auto_optical_isomer_ex_symmetry_arkane
branch
from
614b97b
to
4f1c8c6
Compare
|
@goldmanm, many of Arkane's tests still use the |
goldmanm
force-pushed
the
auto_optical_isomer_ex_symmetry_arkane
branch
from
4f1c8c6
to
39ced9d
Compare
|
@alongd, fixes for the tests should be back up. I was trying to fix a codacy issue and ended up pushing outdated version of the branch. force pushing is dangerous. |
arkane/log.py
Outdated
| symmetry = pg.symmetryNumber | ||
| logging.debug("Symmetry algorithm found {0} optical isomers and a symmetry number of {1}".format(optical_isomers,symmetry)) | ||
| else: | ||
| logging.warning("Symmetry algorithm errored when computing optical isomers. Setting to 1") |
There was a problem hiding this comment.
I think this should be a logging.error level, and the text should tell the user to try re-running and supply the missing values (unless we want to raise an error?)
Also, the current message only refers to opt isomers, symmetry could be problematic as well
There was a problem hiding this comment.
I wonder how making this an error might cause issues when integrating with ARC (or other automated software generation). I guess we just have to integrate the code in a way such that a factor of two error does not destroy the whole RMG run.
There was a problem hiding this comment.
Changed to logging.error
goldmanm
force-pushed
the
auto_optical_isomer_ex_symmetry_arkane
branch
2 times, most recently
from
fb42dd4
to
f1e196f
Compare
goldmanm
force-pushed
the
auto_optical_isomer_ex_symmetry_arkane
branch
from
f1e196f
to
b4c49df
Compare
|
@goldmanm, let's rebase and merge this PR, then do the respective ARC PR |
This commit makes the Log file readers for QChem, Gaussian and Molepro inherit from the base Log class, which can reduce code duplication going forward. To do this, the method `determine_qm_software` was removed from the Log class as this causes circular import errors, and the `Log` class was moved to a separate file, in accordance with PEP-8. Methods used in all child classes are added to the parent Log class. This commit also modified methods calling `determine_qm_software`, and references to `isinstance(x,Log)` were altered to not include the children log files.
Previously, not having the number of opticalIsomers would throw and error. Now the computer can estimate the number of optical isomers, so no input is accepted.
Prevously loadConformer required an integer input for optical isomers. This commit allows this method to call the Symmetry package and estimate the value automatically.
Created new method get_optical_isomers_and_symmetry_number in Log class This method allows Log objects to calculate the number of optical isomers and symmetry numbers contained within a log file. This was taken and modified from the equivalent method in ARC (credits to Alon). Remove symfromlog parameter. If using the RMG QM symmetry package, the symfromlog parameter is not necessary and confusing. This commit removes the need for such a parameter. Get Arkane to use Symmetry package for symmetry. This commit uses the Symmetry package to estimate symmetry if not explicitly given by the user by modifying the gaussian, molpro, and qchem parsers.
Previously, no error was raised when the `linear` parameter was not implemented properly. This could hide information about incorrect calculations if the user is not properly implementing the parameter. This commit raises an error if no `linear` parameter is given.
goldmanm
force-pushed
the
auto_optical_isomer_ex_symmetry_arkane
branch
from
b4c49df
to
009a23c
Compare
Motivation or Problem
Currently Arkane requires optical isomer input.
Description of Changes
This PR allows Arkane to estimate optical isomer and symmetry using the qm symmetry package.
Testing
symfromlogboolean was removed.Reviewer Tips
Run an arkane calculation with & without the symmetry and optical isomer parameter.