Unexpected behavior in Arkane kinetic fitting #1671
Assignees
Labels
Comments
|
I agree we should use consistent 1/T spacing |
goldmanm
added a commit
to goldmanm/RMG-Py
that referenced
this issue
Jul 28, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See ReactionMechanismGenerator#1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead.
goldmanm
added a commit
that referenced
this issue
Jul 28, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See #1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead. Documentation was updated to better describe the three methods of specifying temperature when calling `kinetics` in Arkane
goldmanm
added a commit
that referenced
this issue
Jul 28, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See #1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead. Documentation was updated to better describe the three methods of specifying temperature when calling `kinetics` in Arkane
goldmanm
added a commit
that referenced
this issue
Jul 28, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See #1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead. Documentation was updated to better describe the three methods of specifying temperature when calling `kinetics` in Arkane
goldmanm
added a commit
that referenced
this issue
Jul 30, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See #1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead. Documentation was updated to better describe the three methods of specifying temperature when calling `kinetics` in Arkane
|
I think this issue has been resolved by the merged PR. If I am wrong please feel free to reopen this. |
goldmanm
added a commit
to goldmanm/RMG-Py
that referenced
this issue
Oct 20, 2019
Previously, there were two spots where a list of temperatures was created for fitting high pressure kinetics in Arkane. One fit using inverse temperature spacing and the other one fit with linear temperature spacing. See ReactionMechanismGenerator#1671 This commit moves the location for creating a listing of temperatures to the __init__ method, so there is only one spot where temperatures are initialized. The default min and max in `generateKinetics` was moved into the __init__ method. Three unittests were written for obtaining the proper temperatures in the __init__ method. `generateKinetics` previously accepted an arguement Tlist, which is already an attribute of KineticsJob, creating redundancy. The Tlist argument was removed and self.Tlist was used in this method instead, simplifying the code. Calls to quantity.Quantity in the __init__ method were unnecessary before setting them with quantity.Temperature, so the __init__ code was simplified by setting attributes with (value, 'K') instead. Documentation was updated to better describe the three methods of specifying temperature when calling `kinetics` in Arkane
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug Description
If a user of arkane does not input temperature ranges for high-pressure kinetics, Arkane will space the values between 298 and 2500 K using equal spacing in 1/T (located in generateKinetics of kinetics.py)
If the user specifies Tmin, Tmax, and Tcount, then Arkane will space the points out equally in T space (located in init in kinetics.py).
The difference in spacing also goes against what the original commit message says (beaafb5). I would think that to be consistent, we should either use T or 1/T spacing for both. My personal preference is 1/T spacing. Unless people have opposition, I cam implement this change.
The text was updated successfully, but these errors were encountered: