Skip to content

Catch Bad Conformer ID error from rdkit#2130

Merged
amarkpayne merged 1 commit into
masterfrom
qmfallback
May 14, 2021
Merged

Catch Bad Conformer ID error from rdkit#2130
amarkpayne merged 1 commit into
masterfrom
qmfallback

Conversation

@mjohnson541

Copy link
Copy Markdown
Contributor

So this is a bandaid for #1864 and establishes a way for QM to fallback to ML or GAV when it errors. #1864 needs more investigation, but this PR would at least enable jobs using QM to run sensibly.

@amarkpayne

Copy link
Copy Markdown
Member

If this works I think we can also close #1875

@mjohnson541 mjohnson541 requested review from amarkpayne and removed request for amarkpayne May 14, 2021 16:18
@amarkpayne amarkpayne self-requested a review May 14, 2021 16:27

@amarkpayne amarkpayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two quick changes but otherwise I think this is a good temporary fix until we figure how to make the RDKit conformer generation more robust

Comment thread rmgpy/data/thermo.py Outdated
Comment on lines +1345 to +1347
except ValueError: #rdkit fails to generate conformers
logging.error("Quantum Mechanics calculation failed for species: {0}".format(species.label))
logging.error("Falling back to ML (If turned on) or GAV (If not)")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ValueError can be a lot of things besides the conformer error that raises this ValueError. Just in case I think we should display the original ValueError message--that way if it is something else we can figure it out faster. Can you retain the message and pass it on to the first logging.error statement? Something like Quantum Mechanics calculation failed for species {0} with error {1}?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now

Comment thread rmgpy/data/thermo.py Outdated
else: # Not too many radicals: do a direct calculation.
thermo0 = quantum_mechanics.get_thermo_data(original_molecule) # returns None if it fails
except ValueError: #rdkit fails to generate conformers
logging.error("Quantum Mechanics calculation failed for species: {0}".format(species.label))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change the formatting here for performance. I had to remind myself of the correct syntax here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed now

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi, I just created a new environment and installed RMG just now. I am still stuck with gaussian.

Adding rate rules from training set in kinetics families... Traceback (most recent call last): File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmg.py", line 118, in <module> main() File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmg.py", line 112, in main rmg.execute(**kwargs) File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/main.py", line 673, in execute self.initialize(**kwargs) File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/main.py", line 496, in initialize self.load_database() File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/main.py", line 420, in load_database family.add_rules_from_training(thermo_database=self.database.thermo) File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/data/kinetics/family.py", line 1269, in add_rules_from_training reactant.thermo = thermo_database.get_thermo_data(reactant, training_set=True, metal_to_scale_to=metal) File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/data/thermo.py", line 1344, in get_thermo_data thermo0 = quantum_mechanics.get_thermo_data(original_molecule) # returns None if it fails File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/main.py", line 224, in get_thermo_data thermo0 = qm_molecule_calculator.generate_thermo_data() File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 363, in generate_thermo_data self.qm_data = self.generate_qm_data() File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/gaussian.py", line 254, in generate_qm_data if self.verify_output_file(): File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/gaussian.py", line 170, in verify_output_file qm_data = self.parse() File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 340, in parse qm_data = parse_cclib_data(cclib_data, radical_number + 1) # Should radical_number+1beself.molecule.multiplicity in the next line of code? It's the electronic ground state degeneracy. File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/qmdata.py", line 101, in parse_cclib_data rotational_constants = (cclib_data.rotcons[-1], 'cm^-1') IndexError: list index out of range

Can you help me with this one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@amarkpayne was the cclib version change tested properly for qmtp with Gaussian?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mjohnson541 Actually I should have poster it on the earlier thread.. I just came to this one as a solution. I will post it on the cclib parse error thread as well.. just for historic reasons

@xrulesin xrulesin May 23, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mjohnson541 With the rdkit issue you identified and a qmfallback, you seem to have fixed the issue?

I did get mopac and tried the same and I still stopped at

Reading existing thermo file QMfiles/pm3/BRXCPUJQSHQGEU-UHFFFAOYSA-N.thermo
Traceback (most recent call last):
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmg.py", line 118, in <module>
    main()
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmg.py", line 112, in main
    rmg.execute(**kwargs)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/main.py", line 923, in execute
    trimolecular_react=self.trimolecular_react)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/model.py", line 614, in enlarge
    self.apply_thermo_to_species(procnum)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/rmg/model.py", line 820, in apply_thermo_to_species
    quantum_mechanics.run_jobs(self.new_species_list, procnum=procnum)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/main.py", line 253, in run_jobs
    _write_qm_files_star(qm_arg)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/main.py", line 264, in _write_qm_files_star
    return _write_qm_files(*args)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/main.py", line 271, in _write_qm_files
    quantum_mechanics.get_thermo_data(mol)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/main.py", line 216, in get_thermo_data
    thermo0 = qm_molecule_calculator.generate_thermo_data()
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 363, in generate_thermo_data
    self.qm_data = self.generate_qm_data()
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/mopac.py", line 302, in generate_qm_data
    self.create_geometry()
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 325, in create_geometry
    self.geometry.generate_rdkit_geometries()
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 135, in generate_rdkit_geometries
    self.save_coordinates_from_rdmol(rdmol, min_e_id, rdatom_idx)
  File "/glb/ap/bngsti/proj/ir6/inaveu/sw/rmg/RMG-Py/rmgpy/qm/molecule.py", line 173, in save_coordinates_from_rdmol
    point = rdmol.GetConformer(min_e_id).GetAtomPosition(atom.sorting_label)
ValueError: Bad Conformer Id

and if I look at the rmgpy/data/thermo.py.. I have the following...

1341                         thermo0.S298.value_si -= constants.R * math.log(species.get_symmetry_number())
1342 
1343                     else:  # Not too many radicals: do a direct calculation.
1344                         thermo0 = quantum_mechanics.get_thermo_data(original_molecule)  # returns None if it fails
1345             except ValueError as e: #rdkit fails to generate conformers 
1346                 logging.error("Quantum Mechanics calculation failed for species: %s with ValueError: %s", species.label, e.args[0])
1347                 logging.error("Falling back to ML (If turned on) or GAV (If not)")
1348 
1349         if thermo0 is None:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are you running in parallel? It looks like if you're running in parallel it calls different code and the error doesn't get caught.

@xrulesin xrulesin May 23, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No, I am not running or experimenting right now with parallel options. I am just running it over the linux prompt over with $>rmg.py input.py (unless there is something hidden inside of input.py)...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just to reiterate that I am unable to run the 1,3-hexadiene example with mopac. I am stuck at same "ValueError: Bad Confomer ID). Is it working at your end?

@codecov

codecov Bot commented May 14, 2021

Copy link
Copy Markdown

Codecov Report

Merging #2130 (7656c31) into master (d4e37ac) will increase coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2130      +/-   ##
==========================================
+ Coverage   47.45%   47.48%   +0.02%     
==========================================
  Files          89       89              
  Lines       23949    23953       +4     
  Branches     6237     6237              
==========================================
+ Hits        11365    11374       +9     
+ Misses      11379    11375       -4     
+ Partials     1205     1204       -1     
Impacted Files Coverage Δ
rmgpy/data/thermo.py 67.15% <0.00%> (-0.20%) ⬇️
arkane/kinetics.py 12.24% <0.00%> (ø)
rmgpy/molecule/draw.py 53.52% <0.00%> (+0.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4e37ac...7656c31. Read the comment docs.

…formers

effectively causes fallback to ML or GAV thermo estimation depending on if ML is turned on or not
@mjohnson541

Copy link
Copy Markdown
Contributor Author

Okay, I think I've fixed everything requested and tests have all passed.

@amarkpayne amarkpayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR @mjohnson541 ! I am going to link the issues to close them, and then merge this in

@amarkpayne amarkpayne merged commit 304fabe into master May 14, 2021
@amarkpayne amarkpayne deleted the qmfallback branch May 14, 2021 22:15
@amarkpayne amarkpayne restored the qmfallback branch May 14, 2021 22:15
@amarkpayne amarkpayne deleted the qmfallback branch May 14, 2021 22:15
@xrulesin

xrulesin commented Jun 2, 2021

Copy link
Copy Markdown

I am still crashing on Bad conformer ID while trying RMG-Py/examples/rmg/1,3-hexadiene from shared examples. I did a fresh install of the developer branch from git on 1st June 2021. It is certainly not fixed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cclib/Gaussian tests not passing cclib.parser error when using Gaussian for QMTP

3 participants