Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert 'spin_multiplicity' to 'spinMultiplicity' in Arkane pdep #1798

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

alongd
Copy link
Member

@alongd alongd commented Oct 30, 2019

Motivation or Problem

Running an RMG-generated network file results in:TypeError: transition_state() got an unexpected keyword argument 'spin_multiplicity'.

Description of Changes

Reverted spin_multiplicity to spinMultiplicity in Arkane pdep.py where these networks are being generated. The bug only relates to sting representations of TSs, Species were unaffected.

Testing

Run a RMG with PDep on on master and on this branch, try to execute a sample network file via Arkane.

@alongd alongd requested a review from mliu49 October 30, 2019 15:29
@alongd alongd self-assigned this Oct 30, 2019
@codecov
Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #1798 into master will decrease coverage by 10.31%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1798       +/-   ##
===========================================
- Coverage   42.92%   32.61%   -10.32%     
===========================================
  Files          82       87        +5     
  Lines       21182    26124     +4942     
  Branches     5519     6878     +1359     
===========================================
- Hits         9093     8521      -572     
- Misses      11061    16633     +5572     
+ Partials     1028      970       -58
Impacted Files Coverage Δ
arkane/explorer.py
arkane/encorr/corr.py
arkane/kinetics.py
arkane/encorr/__init__.py
arkane/molpro.py
arkane/orca.py
arkane/log.py
arkane/encorr/data.py
arkane/encorr/mbac.py
arkane/qchem.py
... and 19 more

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 39af849...795bd07. Read the comment docs.

Copy link
Contributor

@mliu49 mliu49 left a comment

Choose a reason for hiding this comment

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

Thanks! Could you rebase?

I don't know if I would consider this a "BigFix" though, lol. Perhaps you could fix the commit message while rebasing.

@alongd
Copy link
Member Author

alongd commented Nov 6, 2019

No problem :)
I'll rebase after we merge #1749

When saving a string representation of a TS in a pdep network file
@alongd alongd changed the title BigFix: Revert 'spin_multiplicity' to 'spinMultiplicity' in Arkane pdep when saving a string representation of a TS in a pdep network file Revert 'spin_multiplicity' to 'spinMultiplicity' in Arkane pdep Nov 6, 2019
@alongd
Copy link
Member Author

alongd commented Nov 6, 2019

rebased and renamed commit

Copy link
Contributor

@mliu49 mliu49 left a comment

Choose a reason for hiding this comment

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

I'm not sure what's up with the Travis PR build. Clicking on the link shows that the job passed.

@mliu49 mliu49 merged commit 01c36ac into master Nov 6, 2019
@mliu49 mliu49 deleted the ts_sm_arkane branch November 6, 2019 21:34
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants