-
Notifications
You must be signed in to change notification settings - Fork 250
A collection of Matt and David's commits from #2316 #2491
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
Changes from all commits
34ed5a3
a6742e9
49bb3c1
7cee142
5badbd1
92fad47
54d9e72
bd1d25d
20adbd1
4425ac5
7d527a3
23d88f2
0a6c456
2180175
9609275
88adf99
f8fdf43
f9991fe
25ff548
409b39d
6071531
09a39e9
4013707
cfe970f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3621,9 +3621,12 @@ def make_bm_rules_from_template_rxn_map(self, template_rxn_map, nprocs=1, Tref=1 | |
| inds = inds.tolist() | ||
| revinds = [inds.index(x) for x in np.arange(len(inputs))] | ||
|
|
||
| pool = mp.Pool(nprocs) | ||
| if nprocs > 1: | ||
| pool = mp.Pool(nprocs) | ||
| kinetics_list = np.array(pool.map(_make_rule, inputs[inds])) | ||
| else: | ||
| kinetics_list = np.array(list(map(_make_rule, inputs[inds]))) | ||
|
|
||
| kinetics_list = np.array(pool.map(_make_rule, inputs[inds])) | ||
| kinetics_list = kinetics_list[revinds] # fix order | ||
|
|
||
| for i, kinetics in enumerate(kinetics_list): | ||
|
|
@@ -4670,3 +4673,61 @@ def _child_make_tree_nodes(family, child_conn, template_rxn_map, obj, T, nprocs, | |
| extension_iter_max=extension_iter_max, extension_iter_item_cap=extension_iter_item_cap) | ||
|
|
||
| child_conn.send(list(family.groups.entries.values())) | ||
|
|
||
| def average_kinetics(kinetics_list): | ||
| """ | ||
| Based on averaging log k. | ||
| Hence we average n, Ea, arithmetically, but we | ||
| average log A (geometric average) | ||
| """ | ||
| logA = 0.0 | ||
| n = 0.0 | ||
| Ea = 0.0 | ||
| count = 0 | ||
| for kinetics in kinetics_list: | ||
| count += 1 | ||
| logA += np.log10(kinetics.A.value_si) | ||
| n += kinetics.n.value_si | ||
| Ea += kinetics.Ea.value_si | ||
|
|
||
| logA /= count | ||
| n /= count | ||
| Ea /= count | ||
|
Comment on lines
+4686
to
+4695
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should just use Python's built in average, or numpy's. i.e.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we'd have to iterate through the list multiple times (once for each attribute) and create a bunch of lists. Is this a verbosity thing? an efficiency thing? a less-likely-to-make-mistakes thing?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can do it very efficiently in one line like this: from types import SimpleNamespace
import numpy as np
# mimic the kinetics objects
kinetics_list = [SimpleNamespace(a=1, b=2, c=3) for _ in range(3)]
a, b, c = np.mean([[np.log10(i.a), i.b, i.c] for i in kinetics_list], axis=1)
This is an antipattern, which we benefit from removing for all of these reasons. Numpy will be faster, less code is easier to maintain and read, etc.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A hole in one! I quite like it. Although, having now taught programming a few times, I am learning to appreciate the benefits of code that a novice can understand at a glance. More participation, fewer mistakes, less time spent explaining or parsing, etc. etc. Anyway, the main reason to not do it your way here and now is that we soon need to revert this commit We can revisit optimizing this code after that happens. |
||
|
|
||
| ## The above could be replaced with something like: | ||
| # logA, n, Ea = np.mean([[np.log10(k.A.value_si), | ||
| # k.n.value_si, | ||
| # k.Ea.value_si] for k in kinetics_list], axis=1) | ||
|
|
||
| Aunits = kinetics_list[0].A.units | ||
| if Aunits in {'cm^3/(mol*s)', 'cm^3/(molecule*s)', 'm^3/(molecule*s)'}: | ||
| Aunits = 'm^3/(mol*s)' | ||
| elif Aunits in {'cm^6/(mol^2*s)', 'cm^6/(molecule^2*s)', 'm^6/(molecule^2*s)'}: | ||
| Aunits = 'm^6/(mol^2*s)' | ||
| elif Aunits in {'s^-1', 'm^3/(mol*s)', 'm^6/(mol^2*s)'}: | ||
| # they were already in SI | ||
| pass | ||
| elif Aunits in {'m^2/(mol*s)', 'cm^2/(mol*s)', 'm^2/(molecule*s)', 'cm^2/(molecule*s)'}: | ||
| # surface: bimolecular (Langmuir-Hinshelwood) | ||
| Aunits = 'm^2/(mol*s)' | ||
| elif Aunits in {'m^5/(mol^2*s)', 'cm^5/(mol^2*s)', 'm^5/(molecule^2*s)', 'cm^5/(molecule^2*s)'}: | ||
| # surface: dissociative adsorption | ||
| Aunits = 'm^5/(mol^2*s)' | ||
| elif Aunits == '': | ||
| # surface: sticking coefficient | ||
| pass | ||
| else: | ||
| raise Exception(f'Invalid units {Aunits} for averaging kinetics.') | ||
|
|
||
| if type(kinetics) not in [Arrhenius,]: | ||
| raise Exception(f'Invalid kinetics type {type(kinetics)!r} for {self!r}.') | ||
|
|
||
| if False: | ||
| pass | ||
| else: | ||
| averaged_kinetics = Arrhenius( | ||
| A=(10 ** logA, Aunits), | ||
| n=n, | ||
| Ea=(Ea * 0.001, "kJ/mol"), | ||
| ) | ||
| return averaged_kinetics | ||
Uh oh!
There was an error while loading. Please reload this page.