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

Minimal Example Does Not Run on Current Master #1674

Closed
amarkpayne opened this issue Aug 7, 2019 · 9 comments · Fixed by #1675
Closed

Minimal Example Does Not Run on Current Master #1674

amarkpayne opened this issue Aug 7, 2019 · 9 comments · Fixed by #1675
Assignees

Comments

@amarkpayne
Copy link
Member

Bug Description

After 22 species have been added into the core, I get the following error:

After model enlargement:
    The model core has 22 species and 55 reactions
    The model edge has 325 species and 1197 reactions


Saving current model core to Chemkin file...
Traceback (most recent call last):
  File "../../../rmg.py", line 181, in <module>
    main()
  File "../../../rmg.py", line 175, in main
    rmg.execute(**kwargs)
  File "/home/ampayne/RMG/.worktrees/RMG-Py-wt2/rmgpy/rmg/main.py", line 874, in execute
    self.saveEverything()
  File "/home/ampayne/RMG/.worktrees/RMG-Py-wt2/rmgpy/rmg/main.py", line 1539, in saveEverything
    self.notify()
  File "/home/ampayne/RMG/.worktrees/RMG-Py-wt2/rmgpy/util.py", line 110, in notify
    observer.update(self)
  File "rmgpy/chemkin.pyx", line 2266, in rmgpy.chemkin.ChemkinWriter.update
    saveChemkinFiles(rmg)
  File "rmgpy/chemkin.pyx", line 2165, in rmgpy.chemkin.saveChemkinFiles
    saveChemkin(rmg.reactionModel,
  File "rmgpy/chemkin.pyx", line 2143, in rmgpy.chemkin.saveChemkin
    saveChemkinFile(path, speciesList, rxnList, verbose = False, checkForDuplicates=False) # We should already have marked everything as duplicates by now
  File "rmgpy/chemkin.pyx", line 1990, in rmgpy.chemkin.saveChemkinFile
    f.write(writeKineticsEntry(rxn, speciesList=species, verbose=verbose))
  File "rmgpy/chemkin.pyx", line 1689, in rmgpy.chemkin.writeKineticsEntry
    assert 0.999 < kinetics.A.getConversionFactorFromSItoCmMolS() / 1.0e6 ** (numReactants - 1) < 1.001, """
AssertionError: 
              Gas phase reaction 
C4H6(46)=C4H6(203)                                  
              with kinetics 
Arrhenius(A=(6.28654e+07,'m^3/(mol*s)'), n=-0.211676, Ea=(0,'kJ/mol'), T0=(1,'K'), Tmin=(300,'K'), Tmax=(1500,'K'), uncertainty=RateUncertainty(mu=0.00579809229276, var=0.287312654976, Tref=1000.0, N=3, correlation='Root_N-1R->H_N-1CClNOSSi->N_N-1COS->O_1CS->C_N-1C-inRing_Ext-2R-R_Ext-3R!H-R_N-Sp-3R!H=2R_Sp-4R!H=3R!H',), comment="""BM rule fitted to 2 training reactions at node Root_N-1R->H_N-1CClNOSSi->N_N-1COS->O_1CS->C_N-1C-inRing_Ext-2R-R_Ext-3R!H-R_N-Sp-3R!H=2R_Sp-4R!H=3R!H
    Total Standard Deviation in ln(k): 1.0891372184
Exact match found for rate rule [Root_N-1R->H_N-1CClNOSSi->N_N-1COS->O_1CS->C_N-1C-inRing_Ext-2R-R_Ext-3R!H-R_N-Sp-3R!H=2R_Sp-4R!H=3R!H]
Euclidian distance = 0
family: R_Recombination""")
              with 1 reactants was expected to have
              kinetics.A.getConversionFactorFromSItoCmMolS() = 1.0
              but instead it is 1000000.0

I am not sure if this is a problem is something just with the Chemkin writer or if it is indicative of a larger problem. I'll do some more debugging soon.

How To Reproduce

Run the minimal example in the examples folder with current master in the database and RMG-Py
(technically I am running this on my isodesmic branch, both database and Py, but both of these branches only affect Arkane and are otherwise update to date with master).

@amarkpayne amarkpayne self-assigned this Aug 7, 2019
@amarkpayne
Copy link
Member Author

amarkpayne commented Aug 7, 2019

Some quick follow-up observations: The reaction family is R_Recombination but I don't think it should be. The species are:

C4H6(46)
CH2 CH C=C(46)

C4H6(203)
C1=CCC1(203)

Shouldn't this be Birad_recombination?

@alongd
Copy link
Member

alongd commented Aug 7, 2019

The R_Recombination family was indeed modified recently

@amarkpayne
Copy link
Member Author

The R_Recombination family was indeed modified recently

I was wondering the same thing. It is probably not a coincidence

@mjohnson541
Copy link
Contributor

Looks like in some cases R_Recombination is now sometimes matching single species as a result of atg4. It's concerning this didn't show up in RMG-tests, do we not have any of these birads in RMG-tests?

@mjohnson541
Copy link
Contributor

Shouldn't be difficult to fix, just need to find the right spot.

@amarkpayne
Copy link
Member Author

@mliu49
Copy link
Contributor

mliu49 commented Aug 7, 2019

We run a modified minimal example with a higher tolerance on RMG-tests. Presumably it doesn't run far enough to add a birad to the core. All of the other tests limit max radicals to 1...

@mliu49
Copy link
Contributor

mliu49 commented Aug 7, 2019

@mjohnson541, I think somewhere around here in __generateReactions you should compare the number of species against the new reactantNum or productNum (depending on direction) and return early if it doesn't match.

@mjohnson541
Copy link
Contributor

@amarkpayne it's a purposeful change, autogenerated trees have one template regardless of the number of reactants.

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

Successfully merging a pull request may close this issue.

4 participants