Compute IDT and IDT SA using Cantera by alongd · Pull Request #150 · ReactionMechanismGenerator/T3

alongd · 2024-05-02T18:02:08Z

Adds Cantera-based Ignition Delay Time (IDT) simulation, RCM adapter, φ-driven fuel/oxidizer/diluent mixture composition, adjoint + brute-force IDT sensitivity analysis, IDT-driven refinement in the main T3 loop, a cleaner restart() control flow, and experimental-data comparison. Plus docs, schema extensions, Cantera-model auto-fix extensions, and tests.

codecov-commenter · 2026-04-12T11:06:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.39%. Comparing base (25e1636) to head (030c585).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
+ Coverage   72.08%   72.39%   +0.30%     
==========================================
  Files          35       37       +2     
  Lines        4678     5741    +1063     
  Branches      998     1245     +247     
==========================================
+ Hits         3372     4156     +784     
- Misses        968     1160     +192     
- Partials      338      425      +87

Flag	Coverage Δ
unittests	`72.39% <ø> (+0.30%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a Cantera-based ignition delay time (IDT) simulation + sensitivity-analysis (SA) pathway, including role-driven fuel/oxidizer/diluent mixtures and schema/docs updates, plus new example inputs and extensive tests.

Changes:

Introduces CanteraIDT (IDT simulation, brute-force + adjoint SA, experimental comparison) and CanteraRCM (constant-pressure reactor variant).
Adds role-based mixture specification (role, equivalence_ratios, oxidizer/diluent parameters) and new sensitivity schema fields for IDT SA.
Improves Cantera model “fix-up” utilities and updates main execution/restart/SA plumbing; adds new tests/data and documentation/examples.

Reviewed changes

Copilot reviewed 22 out of 28 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
tests/test_simulate/test_cantera_IDT.py	New end-to-end and unit tests for IDT simulation, SA utilities, modes, criteria, experiment comparison, and failure handling.
tests/test_schema.py	Adds enum + validation coverage for new schema fields (IDT criterion/method, role fields, max workers).
tests/test_main.py	Updates expected defaults/paths and restart tuple semantics; includes new sensitivity/species/reactor defaults.
tests/test_common.py	Adds tests for new helper utilities (species label cleanup, stoich, φ sweeps).
tests/data/sa_idt_2.yaml	New SA fixture for `get_top_sa_coefficients()` tests.
tests/data/models/eA_units.yaml	New Cantera YAML fixture to validate activation-energy unit parsing.
t3/utils/fix_cantera.py	Adds logging, dedup-mark tracking, and new handling for invalid rate coefficient errors.
t3/simulate/rmg_constant_tp.py	Updates adapter SA API signature to match the new base adapter interface.
t3/simulate/cantera_rcm.py	New adapter subclass using constant-pressure reactors for RCM-like IDT.
t3/simulate/cantera_pfr_t_profile.py	Updates SA API signature forwarding to base.
t3/simulate/cantera_base.py	Updates SA API signature and documents unused params for signature parity.
t3/simulate/cantera_IDT.py	New main Cantera IDT adapter + SA + rate utilities + plotting + experiment comparison.
t3/simulate/adapter.py	Expands abstract SA method signature to include top-N pruning, max workers, save-to-disk flag.
t3/simulate/init.py	Exposes the new `CanteraRCM` adapter.
t3/schema.py	Adds enums + schema fields for IDT SA and role-based φ sweeps; relaxes T/P list length rules for row mode.
t3/runners/rmg_runner.py	Adds optional post-run Cantera model fixing and improves folder creation + error message typo.
t3/main.py	Integrates IDT SA execution, adds IDT SA-based refinement path, updates restart logic, adds figures+SA paths, computes φ sweep concentrations at init.
t3/common.py	Adds helpers for removing numeric suffixes, numpy conversion, atom counts, oxidizer stoich, and φ-driven concentration generation.
examples/idt_with_experiment/input.yml	New example showing IDT SA with experiment comparison wiring.
examples/idt_with_experiment/experimental_idt.yaml	Example experimental IDT YAML format + placeholder data.
docs/input_reference.md	Documents new adapters, new IDT SA fields, role-based mixtures, and `idt_mode`.
docs/examples.md	Adds a new documented example entry for IDT + experiment comparison.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T17:26:57Z

+                if idt is None:
+                    continue


This drops failed working points entirely (idt is None), but the docstring and downstream logic imply None should be stored for failures. Skipping keys makes it impossible to distinguish “failed” from “never simulated”, and can break consumers that expect a fixed T-grid (including plotting/SA alignment). Store None values at idt_dict[phi][P][T] instead of continuing.

Suggested change

if idt is None:

continue

Copilot · 2026-04-13T17:26:58Z

+    concentration = np.asarray([row[0] for row in time_history(radical_label).X], dtype=np.float32)
+    if all(c == 0 for c in concentration):
+        return None
+    dc_dt = np.diff(concentration) / np.diff(times)


Unlike the max_dTdt branch, the radical-based branch divides by np.diff(times) without guarding against zero time steps. If Cantera returns repeated times (or very small dt), this can produce inf/NaN and a bogus IDT index. Apply the same dt = np.where(dt == 0, tiny, dt) pattern here before division.

Suggested change

dc_dt = np.diff(concentration) / np.diff(times)

dt = np.diff(times)

dt = np.where(dt == 0, np.finfo(float).tiny, dt)

dc_dt = np.diff(concentration) / dt

Copilot · 2026-04-13T17:26:58Z

+    i, p_low, p_high = 0, 0.0, 0.0
+    for i in range(len(reaction_data['rate-constants']) - 1):
+        p_low = get_pressure_from_cantera(reaction_data['rate-constants'][i]['P'])
+        p_high = get_pressure_from_cantera(reaction_data['rate-constants'][i + 1]['P'])
+        if p_low <= P <= p_high:
+            break
+    k_low = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i]['A'],
+                                                 n=reaction_data['rate-constants'][i]['b'],
+                                                 Ea=reaction_data['rate-constants'][i]['Ea'],
+                                                 T=T, Ea_units=Ea_units)
+    k_high = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i + 1]['A'],
+                                                  n=reaction_data['rate-constants'][i + 1]['b'],
+                                                  Ea=reaction_data['rate-constants'][i + 1]['Ea'],


For PLOG, if P is outside the tabulated pressure range, the loop never breaks and i ends up at the last segment, causing out-of-range pressures below the minimum to incorrectly interpolate using the highest-pressure entries. Implement the standard behavior: if P <= P_min, use the first rate; if P >= P_max, use the last rate; otherwise interpolate between the bracketing pair.

Suggested change

i, p_low, p_high = 0, 0.0, 0.0

for i in range(len(reaction_data['rate-constants']) - 1):

p_low = get_pressure_from_cantera(reaction_data['rate-constants'][i]['P'])

p_high = get_pressure_from_cantera(reaction_data['rate-constants'][i + 1]['P'])

if p_low <= P <= p_high:

break

k_low = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i]['A'],

n=reaction_data['rate-constants'][i]['b'],

Ea=reaction_data['rate-constants'][i]['Ea'],

T=T, Ea_units=Ea_units)

k_high = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i + 1]['A'],

n=reaction_data['rate-constants'][i + 1]['b'],

Ea=reaction_data['rate-constants'][i + 1]['Ea'],

rate_constants = reaction_data['rate-constants']

pressures = [get_pressure_from_cantera(rate_constant['P']) for rate_constant in rate_constants]

if P <= pressures[0]:

return calculate_arrhenius_rate_coefficient(A=rate_constants[0]['A'],

n=rate_constants[0]['b'],

Ea=rate_constants[0]['Ea'],

T=T, Ea_units=Ea_units)

if P >= pressures[-1]:

return calculate_arrhenius_rate_coefficient(A=rate_constants[-1]['A'],

n=rate_constants[-1]['b'],

Ea=rate_constants[-1]['Ea'],

T=T, Ea_units=Ea_units)

i, p_low, p_high = 0, pressures[0], pressures[1]

for i in range(len(rate_constants) - 1):

p_low = pressures[i]

p_high = pressures[i + 1]

if p_low <= P <= p_high:

break

k_low = calculate_arrhenius_rate_coefficient(A=rate_constants[i]['A'],

n=rate_constants[i]['b'],

Ea=rate_constants[i]['Ea'],

T=T, Ea_units=Ea_units)

k_high = calculate_arrhenius_rate_coefficient(A=rate_constants[i + 1]['A'],

n=rate_constants[i + 1]['b'],

Ea=rate_constants[i + 1]['Ea'],

Copilot · 2026-04-13T17:26:58Z

+        logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxns[0]]})')
+        content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxns[0])


The rxns indices returned by get_dup_rxn_indices() are 1-based (the earlier code marks i - 1). In the removal path you index/remove using rxns[0] without converting to 0-based, so it removes/logs the wrong reaction (off-by-one). Convert to rxns[0] - 1 for both the log access and remove_rxn(..., index=...) call.

Suggested change

logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxns[0]]})')

content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxns[0])

rxn_index = rxns[0] - 1

logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxn_index]})')

content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxn_index)

Copilot · 2026-04-13T17:26:59Z

+        tb (str): The traceback.
+    """
+    content = read_yaml_file(model_path)
+    rxn = get_rxn_to_remove(model_path=model_path, tb=tb)


get_rxn_to_remove() can return None (and also may fail to find an exact equation match). In that case content['reactions'][rxn] raises immediately and prevents the fixer from proceeding. Add a guard: if rxn is None, log the inability to identify the reaction and return without modifying the file.

Suggested change

rxn = get_rxn_to_remove(model_path=model_path, tb=tb)

rxn = get_rxn_to_remove(model_path=model_path, tb=tb)

if rxn is None:

logger.warning('Could not identify the reaction with an invalid rate coefficient; '

'leaving the Cantera file unchanged.')

return

Copilot · 2026-04-13T17:27:00Z

+                future_to_task[future] = (task, task_delta_h, task_delta_k)
+
+            for future in cf.as_completed(future_to_task):
+                task, task_delta_h, task_delta_k = future_to_task[future]
+                try:
+                    idt_dict = future.result()
+                    sa_dict[task[0]]['IDT'][task[1]] = idt_dict
+                    succeeded += 1
+                except Exception as e:
+                    # Retry once with halved perturbation
+                    kind, index = task
+                    retry_delta_h = task_delta_h / 2 if kind == 'thermo' else task_delta_h
+                    retry_delta_k = task_delta_k / 2 if kind == 'kinetics' else task_delta_k
+                    py_logger.warning(f"Task {task} failed: {e}. Retrying with halved perturbation.")
+                    retried += 1
+                    try:
+                        retry_future = executor.submit(worker,
+                                                       task,
+                                                       self.paths['cantera annotated'],
+                                                       self.paths['SA'],
+                                                       self.t3,
+                                                       self.paths,
+                                                       self.rmg,
+                                                       self.logger,
+                                                       1.0,
+                                                       retry_delta_h,
+                                                       retry_delta_k,
+                                                       )
+                        idt_dict = retry_future.result()
+                        sa_dict[task[0]]['IDT'][task[1]] = idt_dict
+                        succeeded += 1
+                    except Exception as e2:
+                        failed += 1
+                        if self.logger is not None:
+                            self.logger.error(f"Task {task} permanently failed after retry:\n{e2}\n"
+                                              f"{traceback.format_exc()}")


The retry path submits a new task but immediately blocks on retry_future.result() inside the as_completed loop, which serializes retries and reduces parallel throughput (especially when multiple tasks fail). Consider collecting retry futures and processing them via as_completed as well, or implement the retry logic inside worker() so each task uses a single process slot until it either succeeds or definitively fails.

Suggested change

future_to_task[future] = (task, task_delta_h, task_delta_k)

for future in cf.as_completed(future_to_task):

task, task_delta_h, task_delta_k = future_to_task[future]

try:

idt_dict = future.result()

sa_dict[task[0]]['IDT'][task[1]] = idt_dict

succeeded += 1

except Exception as e:

# Retry once with halved perturbation

kind, index = task

retry_delta_h = task_delta_h / 2 if kind == 'thermo' else task_delta_h

retry_delta_k = task_delta_k / 2 if kind == 'kinetics' else task_delta_k

py_logger.warning(f"Task {task} failed: {e}. Retrying with halved perturbation.")

retried += 1

try:

retry_future = executor.submit(worker,

task,

self.paths['cantera annotated'],

self.paths['SA'],

self.t3,

self.paths,

self.rmg,

self.logger,

1.0,

retry_delta_h,

retry_delta_k,

)

idt_dict = retry_future.result()

sa_dict[task[0]]['IDT'][task[1]] = idt_dict

succeeded += 1

except Exception as e2:

failed += 1

if self.logger is not None:

self.logger.error(f"Task {task} permanently failed after retry:\n{e2}\n"

f"{traceback.format_exc()}")

future_to_task[future] = (task, task_delta_h, task_delta_k, False)

while future_to_task:

done, _ = cf.wait(future_to_task, return_when=cf.FIRST_COMPLETED)

for future in done:

task, task_delta_h, task_delta_k, is_retry = future_to_task.pop(future)

try:

idt_dict = future.result()

sa_dict[task[0]]['IDT'][task[1]] = idt_dict

succeeded += 1

except Exception as e:

if not is_retry:

# Retry once with halved perturbation

kind, index = task

retry_delta_h = task_delta_h / 2 if kind == 'thermo' else task_delta_h

retry_delta_k = task_delta_k / 2 if kind == 'kinetics' else task_delta_k

py_logger.warning(

f"Task {task} failed: {e}. Retrying with halved perturbation."

)

retried += 1

retry_future = executor.submit(worker,

task,

self.paths['cantera annotated'],

self.paths['SA'],

self.t3,

self.paths,

self.rmg,

self.logger,

1.0,

retry_delta_h,

retry_delta_k,

)

future_to_task[retry_future] = (task, retry_delta_h, retry_delta_k, True)

else:

failed += 1

if self.logger is not None:

self.logger.error(f"Task {task} permanently failed after retry:\n{e}\n"

f"{traceback.format_exc()}")

Copilot · 2026-04-13T17:27:00Z

+                    ax.scatter([1000 / t for t in exp_data[phi][p].keys()],
+                               [e * 1e-6 for e in exp_data[phi][p].values()],
+                               label='experiment', color='orange', marker='D')


The docstrings and example experimental YAML describe IDT units as seconds, but the plot scales experimental values by 1e-6 (microseconds conversion). This will misplot by 1e6 unless the experimental data is actually in µs. Either remove the scaling, or clearly enforce/document that experimental IDTs are in µs and keep the conversion consistent across compare_with_experiment() and docs/examples.

Copilot · 2026-04-13T17:27:01Z

+        visited_species, species_keys = list(), list()
+        visited_rxns, rxn_keys = list(), list()


visited_species mixes two different identifier domains: Cantera species indices (index) and T3 species keys (spc_key). This can cause incorrect dedup behavior (skipping needed work or duplicating work), and can even accidentally collide if the integers overlap. Use separate sets (e.g., visited_ct_species_indices and visited_t3_species_keys) or normalize on one concept (e.g., species labels) throughout this function.

Copilot · 2026-04-13T17:27:01Z

+                                if token == 'thermo':
+                                    if index in visited_species:
+                                        continue
+                                    visited_species.append(index)


visited_species mixes two different identifier domains: Cantera species indices (index) and T3 species keys (spc_key). This can cause incorrect dedup behavior (skipping needed work or duplicating work), and can even accidentally collide if the integers overlap. Use separate sets (e.g., visited_ct_species_indices and visited_t3_species_keys) or normalize on one concept (e.g., species labels) throughout this function.

Copilot · 2026-04-13T17:27:01Z

+                                    for spc in list(reaction.r_species) + list(reaction.p_species):
+                                        spc_key = self.get_species_key(species=spc)
+                                        if spc_key in visited_species:
+                                            continue
+                                        visited_species.append(spc_key)


visited_species mixes two different identifier domains: Cantera species indices (index) and T3 species keys (spc_key). This can cause incorrect dedup behavior (skipping needed work or duplicating work), and can even accidentally collide if the integers overlap. Use separate sets (e.g., visited_ct_species_indices and visited_t3_species_keys) or normalize on one concept (e.g., species labels) throughout this function.

Copilot

Pull request overview

Copilot reviewed 21 out of 27 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-18T18:08:03Z

    for i in rxns:
+        logger.info(f'Removing duplicate mark from reaction {i}:\n({content["reactions"][i]})')
        if 'duplicate' in content['reactions'][i].keys():
-            print(f'Marking reaction {i} as non-duplicate.')
+            logger.info(f'Marking reaction {i} as non-duplicate.')
            del content['reactions'][i]['duplicate']
-    save_yaml_file(model_path, content)
+            changed = True
+        elif 'duplicate' in content['reactions'][i + 1].keys():
+            logger.info(f'Marking reaction {i + 1} as non-duplicate.')
+            del content['reactions'][i + 1]['duplicate']
+            changed = True


fix_no_duplicate_found() treats reaction numbers from get_mistakenly_marked_dup_rxns() as 0-based indices (content['reactions'][i]), but the traceback message “declared duplicate reaction number N” is 1-based (consistent with get_dup_rxn_indices() and the i - 1 usage in fix_undeclared_duplicate_reactions()). This likely clears the wrong reaction’s duplicate flag and can also go out of bounds with i + 1. Convert to 0-based once (idx = i - 1), bounds-check before idx+1, and operate consistently on 0-based list indices.

Copilot · 2026-04-18T18:08:03Z

 def fix_cantera(model_path: str):
    """
    Fix a Cantera model that has incorrectly marked duplicate reactions.
    Creates a backup copy of the Cantera model and fixes the content of the original file in place.

    Args:
        model_path (str): The path to the cantera YAML model file.

    Returns:
        bool: Whether the model was fixed.
    """
+    if not os.path.isfile(model_path):
+        return False
    shutil.copyfile(model_path, model_path + '.bak')
    done, fixed = False, False
+    marked_dups: List[List[int]] = list()
    counter = 0
    while not done and counter < 1000:
        counter += 1
        tb = get_traceback(model_path)
        if tb is None:
            done = True
            break
        else:
            if 'Undeclared duplicate reactions detected' in tb:
-                fix_undeclared_duplicate_reactions(model_path, tb)
-                fixed = True
+                if fix_undeclared_duplicate_reactions(model_path, tb, marked_dups):
+                    fixed = True
+                else:
+                    break
            elif 'No duplicate found for declared duplicate reaction' in tb:
-                fix_no_duplicate_found(model_path, tb)
-                fixed = True
+                if fix_no_duplicate_found(model_path, tb):
+                    fixed = True
+                else:
+                    break
+            elif 'Invalid rate coefficient for reaction' in tb:
+                if remove_reaction_with_invalid_k(model_path, tb):
+                    fixed = True
+                else:
+                    break
            else:
-                print(f'Could not fix {model_path}:\n\n{tb}')
+                logger.warning(f'Could not fix {model_path}:\n\n{tb}')
                break
        time.sleep(1)
    if fixed:
-        print(f'Fixing Cantera model {model_path} (and creating a backup copy with a .bak extension).')
+        logger.info(f'Fixing Cantera model {model_path} (and creating a backup copy with a .bak extension).')
    else:
        os.remove(model_path + '.bak')
    return done


fix_cantera()’s docstring says it returns whether the model was fixed, but it currently returns done (i.e., whether the file loads cleanly after the loop). This makes the return value ambiguous for callers (a model that needed no fixes returns True, and a model that was modified but still doesn’t load returns False). Consider returning fixed (or a tuple like (done, fixed)), and/or update the docstring to match the intended semantics.

Copilot · 2026-04-18T18:08:04Z

+        idt_sa_dict_all = compute_idt_sa(reactor_idt_dict=self.reactor_idt_dict,
+                                         perturbed_idt_dict=sa_dict,
+                                         delta_h=delta_h,
+                                         delta_k=delta_k,
+                                         )


_get_sa_coefficients_brute_force() can submit perturbations with per-species delta_h (adaptive sizing) and also retries failures with halved delta_h/delta_k, but compute_idt_sa() is later called with the original global delta_h/delta_k. That makes the normalized SA coefficients incorrect whenever adaptive sizing or retries occur. Track the actual perturbation used per task (e.g., return it from the worker or keep a task->delta map) and normalize each coefficient with its own delta, or disable delta changes when computing coefficients.

Copilot · 2026-04-18T18:08:04Z

+            new_paths = {**paths, 'cantera annotated': perturbed_model_path}
+            ct_idt_adapter = CanteraIDT(t3=t3, paths=new_paths, rmg=rmg, logger=logger)
+            return ct_idt_adapter.simulate_idt_for_all_reactors(save_yaml=False,
+                                                                save_fig=False,
+                                                                energy='on',
+                                                                max_idt=max_idt)


The worker() constructs CanteraIDT(t3=t3, paths=new_paths, rmg=rmg, logger=logger) without passing the parent adapter’s integration / SA tolerances (atol, rtol, sa_atol, sa_rtol). This can make perturbed-IDT runs inconsistent with the baseline IDT run and skew SA coefficients. Pass these tolerances explicitly (or serialize them into t3['sensitivity'] and read them in __init__) so baseline and perturbed simulations use identical numerical settings.

Copilot · 2026-04-18T18:08:04Z

+    if (idt_index_dc_dt > len(times) - 10
+            or idt < 1e-12
+            or max(concentration) < concentration[0] * 100):


compute_idt() uses max(concentration) < concentration[0] * 100 as a no-ignition filter. If the initial radical mole fraction is 0 (common), this condition becomes max(concentration) < 0, so it never filters anything and tiny numerical noise can be misclassified as ignition. Consider using a nonzero reference such as max(concentration[0], eps) and/or an absolute peak threshold (e.g., max(conc) > 1e-12) in addition to a relative-rise check.

Suggested change

if (idt_index_dc_dt > len(times) - 10

or idt < 1e-12

or max(concentration) < concentration[0] * 100):

peak_concentration = float(np.max(concentration))

reference_concentration = max(float(concentration[0]), np.finfo(float).eps)

if (idt_index_dc_dt > len(times) - 10

or idt < 1e-12

or peak_concentration <= 1e-12

or peak_concentration < reference_concentration * 100):

Updated reference and examples

Added adjoint SA for IDT sim Added the Cantera RCM simulation adapter Added the Cantera PFR T Profile sim adapter

- #1 fix_cantera.get_rxn_to_remove: arity-agnostic multiset matching (no IndexError on unimolecular reactions) - #2 main: remove unused equivalence_ratio_concentrations state - #3 schema: add save_sa_yaml field to T3Sensitivity (knob was unreachable) - #4 cantera_idt: take T/P verbatim in idt_mode='row'; route adjoint SA through _build_combinations (shared row-length guard) - #5 compute_idt: float32 -> float64 for radical concentration - #6 cantera_idt: resize-safe species indexing (time_history.X[:, idx]) to avoid numpy-2.x 'cannot resize' error Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Rebased idt onto main (post py314 merge, PR #171). Integration fixes: - rmg_runner.fix_cantera_model_files: read RMG cantera output from 'cantera_from_ck/' instead of 'cantera/' (main moved it in 9aa5fb0) - migrate IDT test fixtures iteration_{0..4}/RMG/cantera -> cantera_from_ck to match the new layout - modernize idt type hints to PEP 585/604 (main removed the typing imports the idt code relied on, surfacing F821) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 21 out of 27 changed files in this pull request and generated 16 comments.

Comments suppressed due to low confidence (1)

t3/schema.py:67

This return annotation (-> T3Options) is evaluated while the T3Options class body is still executing, so T3Options is not yet bound and importing t3.schema will raise NameError. Use a quoted forward reference (or enable postponed evaluation of annotations for the whole module).

    @model_validator(mode='after')
    def enforce_collision_thermo(self) -> T3Options:
        """
        If collision_violators_rates is True, ensure collision_violators_thermo is also True.
        """
        if self.collision_violators_rates:
            self.collision_violators_thermo = True
        return self

+                ax.set_title(f'IDT vs. 1000/T, phi = {phi}, P = {p:.2f} bar')
+                ax.scatter([1000 / t for t in sim_data.keys()], list(sim_data.values()),
+                           label='simulation', color='blue', marker='o', linestyle='-')
+                ax.set_yscale('log')


+        if success:
+            new_paths = {**paths, 'cantera annotated': perturbed_model_path}
+            ct_idt_adapter = CanteraIDT(t3=t3, paths=new_paths, rmg=rmg, logger=logger)
+            return ct_idt_adapter.simulate_idt_for_all_reactors(save_yaml=False,


+            def _submit(task_, dh_, dk_):
+                return executor.submit(worker,
+                                       task_,
+                                       self.paths['cantera annotated'],
+                                       self.paths['SA'],
+                                       self.t3,
+                                       self.paths,
+                                       self.rmg,
+                                       self.logger,
+                                       1.0,
+                                       dh_,
+                                       dk_,
+                                       )


+                                    if index < 0 or index >= len(self.rmg_reactions):
+                                        continue
+                                    reaction = self.rmg_reactions[index]
+                                    if self.reaction_requires_refinement(reaction=reaction):


 def submit_job(project_directory: str,
-               logger: 'Logger',
+               logger: Logger,
               cluster_soft: str,
               memory: int | None = None,
               ) -> tuple[str | None, str | None]:


+    @model_validator(mode='after')
+    def check_role_consistency(self) -> RMGSpecies:


    @model_validator(mode='after')
-    def check_tolerance_interrupt_simulation(self) -> 'RMGModel':
+    def check_tolerance_interrupt_simulation(self) -> RMGModel:


    @model_validator(mode='after')
-    def check_species_and_reactors(self) -> 'RMG':
+    def check_species_and_reactors(self) -> RMG:


    @model_validator(mode='after')
-    def validate_rmg_t3(self) -> 'InputBase':
+    def validate_rmg_t3(self) -> InputBase:


+        return None
+    arrow = ' <=> ' if ' <=> ' in rxn_str else ' => ' if ' => ' in rxn_str else ' <= '
+    reactants, products = rxn_str.split(arrow)
+    # Match on reactant/product multisets so the order of species (and the arity:


…afe-cyclic-import) t3.common no longer imports t3.chem at all: get_atom_counts builds the molecule from ARCSpecies directly (T3Species is an ARCSpecies subclass), and the T3Species type hints are relaxed to the ARCSpecies base class. This makes the chem<->common dependency one-directional (chem imports to_chemkin_label from common) and clears CodeQL alerts #86 and #87. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ot review) - plot_idt_vs_temperature: use ax.plot (not ax.scatter with linestyle, which draws no connecting line and TypeErrors on some matplotlib versions); sort by temperature for a monotonic 1000/T curve; also catch TypeError so plotting can never crash the IDT run. Regression test added. - _simulate_idt_adjoint_sa: guard the radical d[c]/dt against zero time steps (mirrors compute_idt) to avoid inf/NaN from repeated Cantera times. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…f-review) Second review pass over this PR's own new code. Fixes three bugs introduced earlier in this branch plus one precision nit: - main.py: an interrupted RMG run was silently skipped on restart. The start-iteration RMG guard only checked run_rmg_at_start, so the "RMG began but did not terminate" case (run_rmg=False, restart_rmg=True) never re-ran RMG and T3 proceeded on a truncated model. Extracted T3._should_run_rmg(), which also resumes when restart_rmg is True. - main.py: determine_params_based_on_sa_idt indexed rmg_reactions positionally with the Cantera reaction index, but load_cantera_yaml_file prunes duplicate-equation reactions while Cantera keeps them, so the index spaces diverge once any duplicate exists. Resolve via get_reaction_by_index (matches reaction.index, which equals the full-YAML position). - cantera_idt.py: brute-force SA workers rebuilt CanteraIDT without atol/rtol, using class defaults (1e-16/1e-8) and biasing the finite-difference SA when the model tolerances differ. Thread atol/rtol/sa_atol/sa_rtol through worker(). - cantera_idt.py: the radical early-exit scan still used float32, which loses precision on dilute radicals and could trigger a premature break; use float64. Tests: test_should_run_rmg, test_determine_params_based_on_sa_idt_resolves_index_attr. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

- schema: pydantic v2 str-Enum fields idt_mode (RMGReactor) and role (RMGSpecies) survive model_dump() as enum members, so write_t3_input_file's yaml.dump emits non-safe_load-able !!python/object tags (idt_mode defaults to 'matrix', so it's present in every reactor and broke every all-args input dump). Added @field_serializer for both, matching the existing T3Sensitivity.serialize_enums pattern for idt_criterion/idt_sa_method. - cantera_idt: brute-force SA normalized every coefficient by the scalar base delta_h/delta_k, but the actual perturbation varies per task (adaptive per-species thermo sizing; halved delta on retry), biasing retried/adaptive coefficients. Record the actual delta per (kind, index) and divide by it in compute_idt_sa (scalar fallback preserved). Test test_compute_idt_sa_normalizes_by_actual_delta. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

seed_all_rads (list[RadicalTypeEnum]) survived model_dump() as enum members, so write_t3_input_file's yaml.dump of the schema raised RepresenterError for any species that set it. Same class as the idt_mode/role fix; pre-existing on main (introduced by d5023bd), fixed here opportunistically since this PR already touches schema enum serialization. Added @field_serializer mirroring serialize_role. Test: test_seed_all_rads_serializes_to_plain_strings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

github-actions Bot added examples Module common Module main Module schema Module simulate tests labels May 2, 2024

github-advanced-security AI found potential problems May 2, 2024

View reviewed changes

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

alongd force-pushed the idt branch from 0f13783 to 0216229 Compare July 7, 2024 18:55

alongd force-pushed the idt branch from bf55368 to 7ff2531 Compare November 17, 2024 07:47

github-advanced-security AI found potential problems Nov 17, 2024

View reviewed changes

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread tests/test_simulate/test_cantera_idt.py Fixed

Comment thread tests/test_simulate_adapters/test_cantera_IDT.py Fixed

Comment thread tests/test_simulate_adapters/test_cantera_IDT.py Fixed

alongd force-pushed the idt branch from f2ee256 to a6b61ca Compare December 3, 2024 13:47

github-advanced-security AI found potential problems Dec 3, 2024

View reviewed changes

Comment thread t3/simulate/rmg_constantTP.py Fixed

Comment thread t3/simulate/rxn_idt.py Fixed

Comment thread tests/test_simulate_adapters/test_cantera_IDT.py Fixed

alongd force-pushed the idt branch 4 times, most recently from 8d5b067 to 1b80838 Compare December 4, 2024 14:11

alongd requested a review from calvinp0 March 9, 2025 11:57

alongd force-pushed the idt branch from 74df5b5 to 16fa6e3 Compare April 7, 2026 19:21

github-actions Bot added the Module writer label Apr 7, 2026

alongd removed the request for review from calvinp0 April 8, 2026 17:39

alongd force-pushed the idt branch 2 times, most recently from 4bb147e to 25d4202 Compare April 12, 2026 08:58

github-advanced-security AI found potential problems Apr 12, 2026

View reviewed changes

Comment thread tests/test_simulate/test_cantera_IDT.py Fixed

Comment thread tests/test_simulate/test_cantera_IDT.py Fixed

alongd force-pushed the idt branch from 25d4202 to 4695d41 Compare April 12, 2026 09:02

github-advanced-security AI found potential problems Apr 12, 2026

View reviewed changes

Comment thread t3/simulate/cantera_IDT.py Fixed

Comment thread t3/simulate/cantera_IDT.py Fixed

alongd force-pushed the idt branch 2 times, most recently from d7c81f3 to db5bb3f Compare April 13, 2026 17:14

alongd requested a review from Copilot April 13, 2026 17:18

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Copilot AI reviewed Apr 18, 2026

View reviewed changes

alongd force-pushed the idt branch from 0b1c288 to 9c7195d Compare April 18, 2026 18:17

alongd and others added 17 commits June 7, 2026 00:41

Docs: Added IDT mode and fuel/oxy/diluent examples

fd22d85

Updated reference and examples

Fixed fix_cantera

535a351

Tests: Minor modification to test fix Cantera

9ca2ff1

Use fix cantera in rmg_runner

8d463ff

Added cantera IDT simulate adapter

88e09c7

Added adjoint SA for IDT sim Added the Cantera RCM simulation adapter Added the Cantera PFR T Profile sim adapter

Tests: simulate adapters

47788a8

Added species roles and IDT mode to schema

b3c792a

Tests: schema

98c1e52

Added IDT criterion and SA method to shcema

387b478

Tests: Schema IDT method and SA type

bc26bd4

Added equivalence ratio funcs to common

6ea6c15

Tests: common

fd87826

Adaptations to main

adc6b46

Tests: main

c45f25a

Added IDT with experimental data example

c01fa83

alongd force-pushed the idt branch from 9c7195d to d6d7d30 Compare June 6, 2026 22:05

alongd requested a review from Copilot June 6, 2026 22:12

Copilot started reviewing on behalf of alongd June 6, 2026 22:12 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

alongd and others added 5 commits June 7, 2026 01:20

alongd merged commit 287b92e into main Jun 7, 2026
4 checks passed

alongd deleted the idt branch June 7, 2026 12:53

-    dc_dt = np.diff(concentration) / np.diff(times)
+    dt = np.diff(times)
+    dt = np.where(dt == 0, np.finfo(float).tiny, dt)
+    dc_dt = np.diff(concentration) / dt

-    i, p_low, p_high = 0, 0.0, 0.0
-    for i in range(len(reaction_data['rate-constants']) - 1):
-        p_low = get_pressure_from_cantera(reaction_data['rate-constants'][i]['P'])
-        p_high = get_pressure_from_cantera(reaction_data['rate-constants'][i + 1]['P'])
-        if p_low <= P <= p_high:
-            break
-    k_low = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i]['A'],
-                                                 n=reaction_data['rate-constants'][i]['b'],
-                                                 Ea=reaction_data['rate-constants'][i]['Ea'],
-                                                 T=T, Ea_units=Ea_units)
-    k_high = calculate_arrhenius_rate_coefficient(A=reaction_data['rate-constants'][i + 1]['A'],
-                                                  n=reaction_data['rate-constants'][i + 1]['b'],
-                                                  Ea=reaction_data['rate-constants'][i + 1]['Ea'],
+    rate_constants = reaction_data['rate-constants']
+    pressures = [get_pressure_from_cantera(rate_constant['P']) for rate_constant in rate_constants]
+    if P <= pressures[0]:
+        return calculate_arrhenius_rate_coefficient(A=rate_constants[0]['A'],
+                                                    n=rate_constants[0]['b'],
+                                                    Ea=rate_constants[0]['Ea'],
+                                                    T=T, Ea_units=Ea_units)
+    if P >= pressures[-1]:
+        return calculate_arrhenius_rate_coefficient(A=rate_constants[-1]['A'],
+                                                    n=rate_constants[-1]['b'],
+                                                    Ea=rate_constants[-1]['Ea'],
+                                                    T=T, Ea_units=Ea_units)
+    i, p_low, p_high = 0, pressures[0], pressures[1]
+    for i in range(len(rate_constants) - 1):
+        p_low = pressures[i]
+        p_high = pressures[i + 1]
+        if p_low <= P <= p_high:
+            break
+    k_low = calculate_arrhenius_rate_coefficient(A=rate_constants[i]['A'],
+                                                 n=rate_constants[i]['b'],
+                                                 Ea=rate_constants[i]['Ea'],
+                                                 T=T, Ea_units=Ea_units)
+    k_high = calculate_arrhenius_rate_coefficient(A=rate_constants[i + 1]['A'],
+                                                  n=rate_constants[i + 1]['b'],
+                                                  Ea=rate_constants[i + 1]['Ea'],

		logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxns[0]]})')
		content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxns[0])

-        logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxns[0]]})')
-        content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxns[0])
+        rxn_index = rxns[0] - 1
+        logger.info(f'Removing rxn {rxns[0]} from reaction list:\n({content["reactions"][rxn_index]})')
+        content['reactions'] = remove_rxn(reactions=content['reactions'], index=rxn_index)

-    rxn = get_rxn_to_remove(model_path=model_path, tb=tb)
+    rxn = get_rxn_to_remove(model_path=model_path, tb=tb)
+    if rxn is None:
+        logger.warning('Could not identify the reaction with an invalid rate coefficient; '
+                       'leaving the Cantera file unchanged.')
+        return

		visited_species, species_keys = list(), list()
		visited_rxns, rxn_keys = list(), list()

		@model_validator(mode='after')
		def check_role_consistency(self) -> RMGSpecies:

Conversation

alongd commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

alongd commented May 2, 2024 •

edited

Loading

codecov-commenter commented Apr 12, 2026 •

edited

Loading