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.

Sign up for GitHub

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

Jump to bottom

Several small Pdep bug fixes #1785

Merged
merged 3 commits into from
Oct 30, 2019
Merged

Several small Pdep bug fixes #1785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b2b6ef
BugFix: Incorrect value assignment fixed in me.pyx
alongd Oct 30, 2019
baba248
Don't raise an error if a YAML file does not have all pdep attributed
alongd Oct 30, 2019
c9c1d27
PEP8 style change: `eqRatios` to `eq_ratios` in pdep/network.py
alongd Oct 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions arkane/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,6 @@ def load_yaml(self, path, label=None, pdep=False):
self.imaginary_frequency = ScalarQuantity()
self.imaginary_frequency.make_object(data=freq_data, class_dict=class_dict)

if pdep and not self.is_ts and (self.transport_data is None or self.energy_transfer_model is None):
raise ValueError('Transport data and an energy transfer model must be given if pressure-dependent '
'calculations are requested. Check file {0}'.format(path))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this no longer necessary?

Copy link
Member Author

@alongd alongd Oct 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Can you take a look at the commit message? It explains it

if pdep and not self.is_ts and self.smiles is None and self.adjacency_list is None \
and self.inchi is None and self.molecular_weight is None:
raise ValueError('The molecular weight was not specified, and a structure was not given so it could '
Expand Down
2 changes: 1 addition & 1 deletion rmgpy/pdep/me.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ cpdef generate_full_me_matrix(network, bint products=True):
if k_ij[i, j, n_grains - 1,0] > 0 or k_ij[j, i, n_grains - 1,0] > 0:
for r in range(n_grains):
for s in range(n_j):
u, v = indices[i, r, s], v = indices[j,r,s]
Copy link
Contributor

Choose a reason for hiding this comment

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

How did this not break any tests??

Copy link
Member Author

Choose a reason for hiding this comment

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

right?!? it's very concerning

u, v = indices[i, r, s], indices[j, r, s]
if u > -1 and v > -1:
me_mat[u, v] = k_ij[j, i, r, s]
me_mat[u, u] -= k_ij[j, i, r, s]
Expand Down
10 changes: 5 additions & 5 deletions rmgpy/pdep/network.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class Network(object):
`active_j_rotor` ``True`` if the J-rotor is treated as active, ``False`` if treated as adiabatic
`rmgmode` ``True`` if in RMG mode, ``False`` otherwise
----------------------- ----------------------------------------------------
`eqRatios` An array containing concentration of each isomer and reactant channel present at equilibrium
`coll_freq` An array of the frequency of collision between
`eq_ratios` An array containing concentration of each isomer and reactant channel present at equilibrium
`coll_freq` An array of the frequency of collision between
`Mcoll` Matrix of first-order rate coefficients for collisional population transfer between grains for each isomer
`dens_states` 3D np array of stable configurations, number of grains, and number of J
======================= ====================================================
Expand Down Expand Up @@ -269,7 +269,7 @@ def calculate_rate_coefficients(self, Tlist, Plist, method, error_check=True):
K[t, p, :, :] = self.K

# Check that the k(T,P) values satisfy macroscopic equilibrium
eq_ratios = self.eqRatios
eq_ratios = self.eq_ratios
for i in range(n_isom + n_reac):
for j in range(i):
Keq0 = K[t, p, j, i] / K[t, p, i, j]
Expand Down Expand Up @@ -693,7 +693,7 @@ def calculate_microcanonical_rates(self):
rxn.network_kinetics.get_rate_coefficient(temperature)

# Determine the expected value of the equilibrium constant (Kc)
Keq_expected = self.eqRatios[prod] / self.eqRatios[reac]
Keq_expected = self.eq_ratios[prod] / self.eq_ratios[reac]

# Determine the actual values of k(T) and Keq
C0 = 1e5 / (constants.R * temperature)
Expand Down Expand Up @@ -831,7 +831,7 @@ def calculate_equilibrium_ratios(self):
if self.products[i].has_statmech() or self.products[i].has_thermo():
G = self.products[i].get_free_energy(temperature)
eq_ratios[n_isom + n_reac + i] = math.exp(-G / constants.R / temperature) * conc ** (len(self.products[i].species) - 1)
self.eqRatios = eq_ratios
self.eq_ratios = eq_ratios
return eq_ratios / np.sum(eq_ratios)

def calculate_collision_model(self):
Expand Down