Skip to content

Commit f827297

Browse files
committed
Fix --walltime CLI precedence over input.py settings
Previously, `--walltime` defaulted to `'00:00:00:00'` and was always passed as a kwarg to `RMG.execute()`, silently overriding any `wallTime` set in `input.py`. This meant users who set `wallTime` in their input file and ran without `-t` would have their setting clobbered, giving an effective wall time of zero (no limit). Changes: - Change `--walltime` default to `None`; only add `walltime` and `max_iterations` to the kwargs dict when they are explicitly provided on the command line. Input-file values are now respected by default. - Move walltime/max_iterations override logic earlier in `execute()` (before database loading), so bad values produce an error immediately rather than after a potentially long setup phase. - Extract walltime parsing into a `_parse_walltime_to_seconds()` static method for clarity and testability; improve the error message to include the invalid value. - Log an explicit "Overriding walltime/max_iterations from input file (...) with command-line value (...)" message whenever the CLI does override the input file, making precedence visible in the run log. - Replace bare `return` statements inside nested loops (walltime/maxiter termination paths) with an `end_early` flag + `break`, so that `make_seed_mech()`, `check_model()`, and `finish()` are still called on early termination. Suppress the "MODEL GENERATION COMPLETED" banner when ending early to avoid misleading output. - Update docs (`running.rst`) and example input files to document CLI override behaviour and clarify that `wallTime` can be set in `input.py` while `--maxiter` is CLI-only. - Update test for the new `None` default.
1 parent 9ff1c16 commit f827297

7 files changed

Lines changed: 65 additions & 39 deletions

File tree

documentation/source/users/rmg/running.rst

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ at the command line will print the documentation from ``util.py``, which is repr
4848
-P, --postprocess postprocess profiling statistics from previous
4949
[failed] run; does not run the simulation
5050
-t DD:HH:MM:SS, --walltime DD:HH:MM:SS
51-
set the maximum execution time
51+
set the maximum execution time (overrides input.py if provided)
5252
-i MAXITER, --maxiter MAXITER
53-
set the maximum number of RMG iterations
53+
set the maximum number of RMG iterations (overrides input.py if provided)
5454
-n MAXPROC, --maxproc MAXPROC
5555
max number of processes used during reaction
5656
generation
@@ -73,7 +73,7 @@ Run with multiprocessing for reaction generation and QMTP::
7373

7474
python rmg.py -n <Max number of processes allowed> input.py
7575

76-
Run with setting a limit on the maximum execution time::
76+
Run with setting a limit on the maximum execution time (if specified, then the command-line value overrides any value read from ``input.py``)::
7777

7878
python rmg.py -t <DD:HH:MM:SS> input.py
7979

examples/rmg/MR_test/input.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,9 @@
260260
#Sets a time limit in the form DD:HH:MM:SS after which the RMG job will stop. Useful for profiling on jobs that
261261
#do not converge.
262262
#wallTime = '00:00:00',
263+
#When keepIrreversible=False (default), forces RMG to import library reactions as reversible.
264+
#Otherwise, if set to True, RMG will import library reactions while keeping the reversibility as specified.
263265
keepIrreversible=False,
264-
#Forces RMG to import library reactions as reversible (default). Otherwise, if set to True, RMG will import library
265-
#reactions while keeping the reversibility as as.
266266
)
267267

268268
# optional module allows for correction to unimolecular reaction rates at low pressures and/or temperatures.

examples/rmg/commented/input.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,11 @@
229229
# Helpful for seeing why some reactions are not appearing in the core model.
230230
# Individual writers can override this with their own saveEdge key.
231231
saveEdgeSpecies=False,
232-
# Sets a time limit in the form DD:HH:MM:SS after which the RMG job will stop. Useful for profiling on jobs that
233-
# do not converge.
234-
# wallTime = '00:00:00',
235-
# Forces RMG to import library reactions as reversible (default). Otherwise, if set to True, RMG will import library
236-
# reactions while keeping the reversibility as as.
232+
# Sets a time limit in the form DD:HH:MM:SS after (or shortly before) which the RMG job will stop.
233+
# Useful for profiling on jobs that do not converge.
234+
wallTime = '00:00:00:00',
235+
# If keepIrreversible=False (default) forces RMG to import library reactions as reversible.
236+
# If set to True, RMG will import library reactions while keeping the reversibility as specified.
237237
keepIrreversible=False,
238238
# Allows families with three products to react in the diverse direction (default).
239239
trimolecularProductReversible=True,

rmgpy/__main__.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ def main():
6262

6363
kwargs = {
6464
'restart': args.restart,
65-
'walltime': args.walltime,
6665
'maxproc': args.maxproc,
6766
'kineticsdatastore': args.kineticsdatastore,
68-
'max_iterations': args.maxiter,
6967
}
68+
if args.walltime is not None:
69+
kwargs['walltime'] = args.walltime
70+
if args.maxiter is not None:
71+
kwargs['max_iterations'] = args.maxiter
7072

7173
if args.profile:
7274
import cProfile

rmgpy/rmg/main.py

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,19 @@ def clear(self):
271271
self.exec_time = []
272272
self.liquid_volumetric_mass_transfer_coefficient_power_law = None
273273

274+
@staticmethod
275+
def _parse_walltime_to_seconds(walltime):
276+
"""
277+
Convert walltime string DD:HH:MM:SS to seconds.
278+
"""
279+
data = walltime.split(":")
280+
if len(data) != 4:
281+
raise ValueError("Invalid format for wall time {0}; should be DD:HH:MM:SS.".format(walltime))
282+
try:
283+
return int(data[-1]) + 60 * int(data[-2]) + 3600 * int(data[-3]) + 86400 * int(data[-4])
284+
except ValueError as exc:
285+
raise ValueError("Invalid format for wall time {0}; should be DD:HH:MM:SS.".format(walltime)) from exc
286+
274287
def load_input(self, path=None):
275288
"""
276289
Load an RMG job from the input file located at `input_file`, or
@@ -582,6 +595,24 @@ def initialize(self, **kwargs):
582595
)
583596
)
584597

598+
if "walltime" in kwargs:
599+
logging.info(
600+
"Overriding walltime from input file (%s) with command-line value (%s).",
601+
self.walltime,
602+
kwargs["walltime"],
603+
)
604+
self.walltime = kwargs["walltime"]
605+
606+
if "max_iterations" in kwargs:
607+
logging.info(
608+
"Overriding max_iterations from input file (%s) with command-line value (%s).",
609+
self.max_iterations,
610+
kwargs["max_iterations"],
611+
)
612+
self.max_iterations = kwargs["max_iterations"]
613+
614+
self.walltime = self._parse_walltime_to_seconds(self.walltime)
615+
585616
# Auto-select libraries if any field uses 'auto' or '<PAH_libs>'
586617
auto_select_libraries(self)
587618

@@ -659,21 +690,6 @@ def initialize(self, **kwargs):
659690
if reaction_system.T:
660691
reaction_system.viscosity = solvent_data.get_solvent_viscosity(reaction_system.T.value_si)
661692

662-
try:
663-
self.walltime = kwargs["walltime"]
664-
except KeyError:
665-
pass
666-
667-
try:
668-
self.max_iterations = kwargs["max_iterations"]
669-
except KeyError:
670-
pass
671-
672-
data = self.walltime.split(":")
673-
if not len(data) == 4:
674-
raise ValueError("Invalid format for wall time {0}; should be DD:HH:MM:SS.".format(self.walltime))
675-
self.walltime = int(data[-1]) + 60 * int(data[-2]) + 3600 * int(data[-3]) + 86400 * int(data[-4])
676-
677693
# Initialize reaction model
678694

679695
for spec in self.initial_species:
@@ -931,6 +947,7 @@ def execute(self, initialize=True, **kwargs):
931947
self.make_seed_mech()
932948

933949
max_num_spcs_hit = False # default
950+
end_early = False
934951

935952
for q, model_settings in enumerate(self.model_settings_list):
936953
if len(self.simulator_settings_list) > 1:
@@ -940,7 +957,7 @@ def execute(self, initialize=True, **kwargs):
940957

941958
self.filter_reactions = model_settings.filter_reactions
942959

943-
logging.info("Beginning model generation stage {0}...\n".format(q + 1))
960+
logging.info(f"Beginning model generation stage {q + 1} of {len(self.model_settings_list)}.\n")
944961

945962
self.done = False
946963

@@ -1235,7 +1252,8 @@ def execute(self, initialize=True, **kwargs):
12351252
core_spec, core_reac, edge_spec, edge_reac = self.reaction_model.get_model_size()
12361253
logging.info("The current model core has %s species and %s reactions" % (core_spec, core_reac))
12371254
logging.info("The current model edge has %s species and %s reactions" % (edge_spec, edge_reac))
1238-
return
1255+
end_early = True
1256+
break
12391257

12401258
if self.max_iterations and (self.reaction_model.iteration_num >= self.max_iterations):
12411259
logging.info("MODEL GENERATION TERMINATED")
@@ -1246,12 +1264,16 @@ def execute(self, initialize=True, **kwargs):
12461264
core_spec, core_reac, edge_spec, edge_reac = self.reaction_model.get_model_size()
12471265
logging.info("The current model core has %s species and %s reactions" % (core_spec, core_reac))
12481266
logging.info("The current model edge has %s species and %s reactions" % (edge_spec, edge_reac))
1249-
return
1267+
end_early = True
1268+
break
12501269

12511270
if max_num_spcs_hit: # resets maxNumSpcsHit and continues the settings for loop
12521271
logging.info("The maximum number of species ({0}) has been hit, Exiting stage {1} ...".format(model_settings.max_num_species, q + 1))
12531272
max_num_spcs_hit = False
12541273

1274+
if end_early: # breaks the settings for loop
1275+
break
1276+
12551277
# Save the final seed mechanism
12561278
self.make_seed_mech()
12571279

@@ -1333,12 +1355,14 @@ def execute(self, initialize=True, **kwargs):
13331355

13341356
self.check_model()
13351357
# Write output file
1336-
logging.info("")
1337-
logging.info("MODEL GENERATION COMPLETED")
1338-
logging.info("")
1339-
core_spec, core_reac, edge_spec, edge_reac = self.reaction_model.get_model_size()
1340-
logging.info("The final model core has %s species and %s reactions" % (core_spec, core_reac))
1341-
logging.info("The final model edge has %s species and %s reactions" % (edge_spec, edge_reac))
1358+
1359+
if not end_early:
1360+
logging.info("")
1361+
logging.info("MODEL GENERATION COMPLETED")
1362+
logging.info("")
1363+
core_spec, core_reac, edge_spec, edge_reac = self.reaction_model.get_model_size()
1364+
logging.info("The final model core has %s species and %s reactions" % (core_spec, core_reac))
1365+
logging.info("The final model edge has %s species and %s reactions" % (edge_spec, edge_reac))
13421366

13431367
self.finish()
13441368

rmgpy/util.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def parse_command_line_arguments(command_line_args=None):
173173
parser.add_argument('-P', '--postprocess', action='store_true',
174174
help='postprocess profiling statistics from previous [failed] run; does not run the simulation')
175175

176-
parser.add_argument('-t', '--walltime', type=str, nargs=1, default='00:00:00:00',
176+
parser.add_argument('-t', '--walltime', type=str, nargs=1, default=None,
177177
metavar='DD:HH:MM:SS', help='set the maximum execution time')
178178

179179
parser.add_argument('-i', '--maxiter', type=int, nargs=1, default=None,
@@ -197,7 +197,7 @@ def parse_command_line_arguments(command_line_args=None):
197197
args.file = args.file[0]
198198

199199
# If walltime was specified, retrieve this string from the element 1 list
200-
if args.walltime != '00:00:00:00':
200+
if args.walltime:
201201
args.walltime = args.walltime[0]
202202

203203
if args.restart:

test/rmgpy/rmg/rmgTest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ def test_parse_command_line_arguments_defaults(self):
206206
args = parse_command_line_arguments(["input.py"])
207207

208208
# Test default values
209-
assert args.walltime == "00:00:00:00"
209+
assert args.walltime is None
210210
assert args.output_directory == os.path.abspath(os.path.dirname("./"))
211211
assert args.debug == False
212212
assert args.file == "input.py"

0 commit comments

Comments
 (0)