Merge pull request #167 from ReactionMechanismGenerator/readme3

alongd · web-flow · commit 6ca7a1092cc4 · 2026-04-12T07:36:16.000+03:00
Various repo-wide fixes
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -2,12 +2,18 @@ name: T3 CI
 
 on:
   workflow_dispatch:
+  push:
+    branches: [main]
   pull_request:
     branches: [main]
     types: [opened, synchronize, reopened]
   schedule:
     - cron: '0 0 * * *'
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
 jobs:
   lint:
     name: Lint (ruff)
diff --git a/.vscode/launch.json b/.vscode/launch.json
diff --git a/.vscode/settings.json b/.vscode/settings.json
diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md
@@ -1,4 +1,4 @@
-# The RMG-ARC Tandem Tool (T3) Code of Conduct
+# T3 Code of Conduct
 
 ## Our Pledge
 
diff --git a/Makefile b/Makefile
@@ -10,7 +10,7 @@ test:
 	pytest tests/ --cov -ra -vv
 
 test-main:
-	pytest tests/test_main.py -ra -vv
+	pytest tests/test_main.py --cov -ra -vv
 
 test-functional:
-	pytest tests/test_functional.py -ra -vv
+	pytest tests/test_functional.py --cov -ra -vv
diff --git a/README.md b/README.md
@@ -1,8 +1,3 @@
-<p align="center">
-  <a href="https://reactionmechanismgenerator.github.io/T3/"><img src="https://github.com/ReactionMechanismGenerator/T3/blob/main/grf/T3_logo_small.gif" alt="T3"></a>
-</p>
-
-# The Tandem Tool (T3) for automated chemical kinetic model development
 
 [![CI](https://github.com/ReactionMechanismGenerator/T3/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/ReactionMechanismGenerator/T3/actions/workflows/ci.yml)
 [![Docs](https://github.com/ReactionMechanismGenerator/T3/actions/workflows/gh-pages.yml/badge.svg)](https://reactionmechanismgenerator.github.io/T3/)
@@ -11,6 +6,8 @@
 [![MIT license](http://img.shields.io/badge/license-MIT-brightgreen.svg)](http://opensource.org/licenses/MIT)
 ![python](https://img.shields.io/badge/Python-3.12+-blue.svg)
 
+# The Tandem Tool (T3) for automated chemical kinetic model development
+
 **T3** automates the development of detailed chemical kinetic models.
 Given a set of initial species and conditions, it produces a validated model with
 high-fidelity thermochemistry and rate parameters by iteratively combining
diff --git a/pyproject.toml b/pyproject.toml
@@ -38,6 +38,12 @@ ignore = [
     "W291",   # trailing whitespace
 ]
 
+[tool.pytest.ini_options]
+filterwarnings = [
+    "ignore::DeprecationWarning",
+    "ignore::PendingDeprecationWarning",
+]
+
 [tool.ruff.lint.per-file-ignores]
 "__init__.py" = ["F401"]  # unused imports in __init__.py are re-exports
 "t3/runners/rmg_incore_sa.py" = ["E701"]  # subprocess script, compact one-liners are fine
diff --git a/pytest.ini b/pytest.ini
diff --git a/t3/main.py b/t3/main.py
@@ -582,6 +582,7 @@ def run_rmg(self, restart_rmg: bool = False):
                                                t3_project_name=self.project,
                                                rmg_execution_type=self.rmg['rmg_execution_type'],
                                                restart_rmg=restart_rmg,
+                                               walltime=self.t3['options']['max_RMG_walltime'],
                                                )
         if rmg_exception_encountered:
             self.rmg_exceptions_counter += 1
@@ -1151,13 +1152,13 @@ def get_reaction_key(self,
                     try:
                         if label == t3_reaction.get_reaction_smiles_label():
                             return key
-                    except Exception:
+                    except (AttributeError, ValueError):
                         pass
                 elif label_type == 'Chemkin':
                     try:
                         if label == t3_reaction.to_chemkin():
                             return key
-                    except Exception:
+                    except (AttributeError, ValueError):
                         pass
         return None
 
diff --git a/t3/runners/rmg_runner.py b/t3/runners/rmg_runner.py
@@ -4,9 +4,11 @@
 """
 
 import datetime
+import logging
 import os
 import shlex
 import shutil
+import subprocess
 import time
 from typing import TYPE_CHECKING, List, Optional, Tuple
 
@@ -161,9 +163,27 @@ def rmg_job_converged(project_directory: str) -> Tuple[bool, Optional[str]]:
     return rmg_converged, error
 
 
+_DEFAULT_RMG_TIMEOUT_S = 6 * 3600  # 6 hours
+
+logger = logging.getLogger(__name__)
+
+
+def _parse_walltime_to_seconds(walltime: str) -> int:
+    """Parse a 'DD:HH:MM:SS' walltime string to total seconds. Returns 0 for '00:00:00:00'."""
+    parts = walltime.split(':')
+    if len(parts) != 4:
+        return 0
+    try:
+        days, hours, minutes, seconds = (int(p) for p in parts)
+    except ValueError:
+        return 0
+    return days * 86400 + hours * 3600 + minutes * 60 + seconds
+
+
 def run_rmg_incore(rmg_input_file_path: str,
                    verbose: Optional[int] = None,
                    max_iterations: Optional[int] = None,
+                   walltime: Optional[str] = None,
                    ) -> bool:
     """
     Run RMG incore under the rmg_env.
@@ -172,10 +192,14 @@ def run_rmg_incore(rmg_input_file_path: str,
         rmg_input_file_path (str): The path to the RMG input file.
         max_iterations (int, optional): Max RMG iterations.
         verbose (int, optional): Level of verbosity.
+        walltime (str, optional): Max walltime in 'DD:HH:MM:SS' format. Defaults to 6 hours.
 
     Returns:
         bool: Whether an exception was raised.
     """
+    timeout_s = _parse_walltime_to_seconds(walltime) if walltime else 0
+    if timeout_s <= 0:
+        timeout_s = _DEFAULT_RMG_TIMEOUT_S
     project_directory = os.path.abspath(os.path.dirname(rmg_input_file_path))
     verbose = f' -v {verbose}' if verbose is not None else ''
     max_iterations = f' -m {max_iterations}' if max_iterations is not None else ''
@@ -192,8 +216,16 @@ def run_rmg_incore(rmg_input_file_path: str,
     echo "Micromamba/Mamba/Conda required" >&2
     exit 1
 fi' '''
-    stdout, stderr = execute_command(shell_script, shell=True, no_fail=True, executable='/bin/bash')
-    stderr_text = ''.join(stderr) if isinstance(stderr, list) else (stderr or '')
+    try:
+        result = subprocess.run(shell_script, shell=True, executable='/bin/bash',
+                                capture_output=True, text=True, timeout=timeout_s)
+        stderr_text = result.stderr or ''
+    except subprocess.TimeoutExpired:
+        logger.error(f'RMG incore timed out after {timeout_s}s')
+        return True
+    if result.returncode != 0:
+        logger.error(f'RMG incore exited with code {result.returncode}')
+        return True
     if 'RMG threw an exception and did not converge.' in stderr_text:
         return True
     return False
@@ -271,6 +303,7 @@ def rmg_runner(rmg_input_file_path: str,
                t3_project_name: Optional[str] = None,
                rmg_execution_type: Optional[str] = None,
                restart_rmg: bool = False,
+               walltime: Optional[str] = None,
                ) -> bool:
     """
     Run an RMG job as a subprocess under the rmg_env.
@@ -286,6 +319,7 @@ def rmg_runner(rmg_input_file_path: str,
         t3_project_name (str, optional): The T3 project name, used for setting a job name on the server for the RMG run.
         rmg_execution_type (str, optional): The RMG execution type (incore or local). Also set via settings.py.
         restart_rmg (bool, optional): Whether to restart RMG from seed.
+        walltime (str, optional): Max walltime in 'DD:HH:MM:SS' format. Defaults to 6 hours.
 
     Returns:
         bool: Whether an exception was raised.
@@ -299,6 +333,7 @@ def rmg_runner(rmg_input_file_path: str,
         rmg_exception_encountered = run_rmg_incore(rmg_input_file_path=rmg_input_file_path,
                                                    verbose=verbose,
                                                    max_iterations=max_iterations,
+                                                   walltime=walltime,
                                                    )
         return rmg_exception_encountered
     elif rmg_execution_type == 'local':
diff --git a/t3/schema.py b/t3/schema.py
@@ -477,7 +477,7 @@ class Config:
     @classmethod
     def check_units(cls, value):
         """RMGOptions.units validator"""
-        if value.lower() is not None and value != 'si':
+        if value.lower() != 'si':
             raise ValueError(f'Currently RMG only supports SI units, got "{value}"')
         return value.lower()
 
diff --git a/t3/simulate/cantera_base.py b/t3/simulate/cantera_base.py
@@ -8,6 +8,7 @@
 different reactor configuration.
 """
 
+import logging
 import cantera as ct
 import numpy as np
 from abc import abstractmethod
@@ -331,6 +332,9 @@ def simulate(self):
                     non_inert_mask = np.ones(self.num_ct_species, dtype=bool)
                     non_inert_mask[self.inert_index_list] = False
 
+                    if not np.all(np.isfinite(mass_frac_sa)):
+                        logging.getLogger(__name__).warning(
+                            'NaN/inf detected in mass fraction SA matrix — check for numerical issues')
                     kin_correction = mass_frac_sa[non_inert_mask, :self.num_ct_reactions].sum(axis=0)
                     thermo_correction = mass_frac_sa[non_inert_mask, self.num_ct_reactions:].sum(axis=0)
 
@@ -462,9 +466,17 @@ def get_idt_by_T(self):
             time, data_list, reaction_sensitivity_data, thermodynamic_sensitivity_data = condition_data
             T_data = data_list[0]
 
-            dTdt = np.diff(T_data.data) / np.diff(time.data)
-            idt_dict['idt_index'].append(int(np.argmax(dTdt)))
-            idt_dict['idt'].append(time.data[idt_dict['idt_index'][i]])
+            dt = np.diff(time.data)
+            dt = np.where(dt == 0, np.finfo(float).tiny, dt)
+            dTdt = np.diff(T_data.data) / dt
+            valid = np.isfinite(dTdt)
+            if valid.any():
+                masked = np.where(valid, dTdt, -np.inf)
+                idx = int(np.argmax(masked))
+            else:
+                idx = 0
+            idt_dict['idt_index'].append(idx)
+            idt_dict['idt'].append(time.data[min(idx + 1, len(time.data) - 1)])
 
         return idt_dict
 
diff --git a/t3/simulate/cantera_jsr.py b/t3/simulate/cantera_jsr.py
@@ -114,6 +114,8 @@ def reinitialize_simulation(self, T0=None, P0=None, X0=None, V0=None):
         self._jsr_exhaust = ct.Reservoir(self.model)
         self.cantera_reactor = ct.IdealGasReactor(self.model, energy='off', volume=VOLUME)
 
+        if residence_time <= 0:
+            raise ValueError(f'Invalid residence time: {residence_time}')
         self._jsr_mfc = ct.MassFlowController(
             upstream=self._jsr_inlet,
             downstream=self.cantera_reactor,
diff --git a/t3/simulate/cantera_pfr.py b/t3/simulate/cantera_pfr.py
@@ -129,6 +129,7 @@ def _compute_distance(self):
             # Velocity: u = u_0 * mean_MW_0 / mean_MW  (P, T, A all cancel)
             total_residence_time = self.conditions[idx].reaction_time.value_si
             u_0 = LENGTH / total_residence_time
+            mean_MW = np.where(mean_MW > 0, mean_MW, np.finfo(float).tiny)
             u = u_0 * mean_MW[0] / mean_MW
 
             # Axial distance [m] via cumulative integration
@@ -203,6 +204,8 @@ def _simulate_chain(self):
                 sim.advance_to_steady_state()
 
                 # Residence time in this cell
+                if mass_flow_rate <= 0:
+                    raise ValueError(f'Invalid mass flow rate: {mass_flow_rate}')
                 cell_residence_time = reactor.mass / mass_flow_rate
                 cumulative_time += cell_residence_time
 
diff --git a/t3/simulate/cantera_pfr_t_profile.py b/t3/simulate/cantera_pfr_t_profile.py
@@ -107,6 +107,11 @@
 T_HOT = 900.0          # K, isothermal plateau temperature
 T_OUTLET = 500.0       # K, outlet temperature
 
+if not (0 < RAMP_UP_END < ISO_END < LENGTH):
+    raise ValueError(
+        f'Temperature profile constants must satisfy 0 < RAMP_UP_END ({RAMP_UP_END}) < ISO_END ({ISO_END}) < LENGTH ({LENGTH})'
+    )
+
 
 def temperature_profile(z):
     """
@@ -307,7 +312,11 @@ def simulate(self):
                 # Local velocity at the new (T, rho) and the corresponding
                 # time step to traverse one segment.
                 rho = self.model.density
+                if rho * AREA <= 0:
+                    raise ValueError(f'Invalid density ({rho}) or area ({AREA}) in PFR T-profile cell {n}')
                 u = mass_flow_rate / (rho * AREA)
+                if u <= 0:
+                    raise ValueError(f'Invalid velocity ({u}) in PFR T-profile cell {n}')
                 dt = dz / u
 
                 sim.advance(sim.time + dt)
diff --git a/t3/simulate/rmg_constant_tp.py b/t3/simulate/rmg_constant_tp.py
@@ -5,6 +5,7 @@
 
 import datetime
 import itertools
+import logging
 import os
 import numpy as np
 from typing import List, Optional, TYPE_CHECKING
@@ -279,6 +280,12 @@ def get_species_concentration_lists_from_ranged_params(self) -> List[List[dict]]
                 species_lists.append(new_species_list)
 
         for species_list in species_lists:
+            total = sum(s['concentration'] for s in species_list
+                        if isinstance(s['concentration'], (int, float)))
+            if total > 1.0 or total <= 0:
+                logging.getLogger(__name__).warning(
+                    f'Species concentrations sum to {total:.4f}, expected (0, 1]. '
+                    f'Cantera will normalize mole fractions.')
             species_list.sort(key=lambda spc: spc['concentration'][0] if isinstance(spc['concentration'], (tuple, list))
             else spc['concentration'], reverse=True)
         return species_lists
diff --git a/t3/utils/libraries.py b/t3/utils/libraries.py
@@ -21,8 +21,8 @@
     from t3.logger import Logger
 
 
-_LOCK_TIMEOUT_S = 3600  # 1 hour wait for lock acquisition
-_LOCK_STALE_S = 3600    # 1 hour before breaking a stale lock
+_LOCK_TIMEOUT_S = 300   # 5 min wait for lock acquisition
+_LOCK_STALE_S = 300     # 5 min before breaking a stale lock
 _LOCK_POLL_S = 10.0     # Poll every 10 seconds
 
 
diff --git a/t3/utils/rmg_sa_parser.py b/t3/utils/rmg_sa_parser.py
@@ -93,7 +93,10 @@ def parse_rmg_sa_csv(csv_path: str) -> Tuple[List[float], Dict[str, Dict[int, Li
                 value = float(row[col_idx])
                 if kind == 'kinetics':
                     # param_raw is e.g. 'k10' -> extract integer 10
-                    rxn_idx = int(param_raw.lstrip('k'))
+                    try:
+                        rxn_idx = int(param_raw.lstrip('k'))
+                    except ValueError:
+                        continue
                     kinetics.setdefault(obs_label, {}).setdefault(rxn_idx, []).append(value)
                 else:
                     spc_label = _strip_rmg_index(param_raw)
diff --git a/t3/utils/rmg_shim.py b/t3/utils/rmg_shim.py
@@ -5,6 +5,7 @@
 in-process dependency on rmgpy itself.
 """
 
+import logging
 import os
 import tempfile
 from dataclasses import dataclass, field
@@ -690,6 +691,9 @@ def write_atomic(path: str, content: str) -> None:
             f.write(content)
         os.replace(tmp_path, path)
     except Exception:
-        if os.path.exists(tmp_path):
-            os.unlink(tmp_path)
+        try:
+            if os.path.exists(tmp_path):
+                os.unlink(tmp_path)
+        except OSError:
+            logging.getLogger(__name__).warning(f'Failed to clean up temp file: {tmp_path}')
         raise
diff --git a/t3/utils/writer.py b/t3/utils/writer.py
diff --git a/tests/test_main.py b/tests/test_main.py

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-# The RMG-ARC Tandem Tool (T3) Code of Conduct`
	`1`	`+# T3 Code of Conduct`
`2`	`2`
`3`	`3`	`## Our Pledge`
`4`	`4`