diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index f05ce4ddd..9652fbc1b 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -586,12 +586,16 @@ jobs: # Re-uses the default libtotapi.so artifact built earlier in # this job (the "Build default libtotapi.so" step landed in # PR #207) for test_non_mono_so_raises. + # test_set_after_run.py (#253) is listed here as well so its + # MONO_LIB_PATH-gated fp / ti classes run on the mono image; + # its TestTotSetAfterRun class also runs in the main pytest step. env: MONO_LIB_PATH: ${{ github.workspace }}/tot/libtotapi_mono.so TOTLIB_PATH: ${{ github.workspace }}/tot/libtotapi.so PYTHONPATH: python run: | python -m pytest python/totlib/tests/test_mono_routing.py \ + python/totlib/tests/test_set_after_run.py \ --forked --timeout=120 --timeout-method=signal -v - name: Mono pipeline coupling (Phase 2c PR-B) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0df80e3b..495034f4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ The format is loosely based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- **tr / fp / ti libraries, `Tot` and `tot_mcp`: a parameter set after a + run re-preps the module, whoever set it (#253).** `tr_set_param` / + `fp_set_param` / `ti_set_param` used to clear the module's private + "prepared" flag themselves; tot's `tr:` / `fp:` / `ti:` dispatcher wrote + to the registries directly and left the flag standing, so after a run a + `Tot.set_param("tr:NSMAX", 3)` continued the old state with `tr_prep` + skipped and the #189 pre-run species check never fired. The registries + now count every successful set and `tr_run` / `fp_run` / `ti_run` + re-prep when the count moved since their last prep. Behaviour change for + `Tot` and `tot_mcp` only: a set after a run now restarts the module + (`NT` and `T` back to 0), which is what `trlib` / `fplib` / `tilib` and + `TotPipeline` have always done; a set the registry refused changes + nothing. `eq` has no prepared flag on `develop`; the same counter is the + natural precondition for `eq_save` (#256). + - **FP library: `fp_run` refuses `NRMAX` above `FP_MAX_NRMAX` (100).** Such a run was made, and `fp_get_state` then returned rc=3: its results could be read only from the log on unit 6 or from `fp_regress.dat` diff --git a/fp/fp_api.f90 b/fp/fp_api.f90 index f4f9a6939..7bb1b7148 100644 --- a/fp/fp_api.f90 +++ b/fp/fp_api.f90 @@ -35,7 +35,7 @@ MODULE fp_api NRMAX, NSAMAX, NPMAX, NTHMAX, NTG2, TIMEFP, & NTMAX, & RNT, RWT, RTT, RJT, RPCT, RPWT - USE fp_param_registry, ONLY: fp_param_set, fp_param_set_str + USE fp_param_registry, ONLY: fp_param_set, fp_param_set_str, fp_param_generation USE plinit, ONLY: pl_init USE equnit, ONLY: eq_init USE obinit, ONLY: ob_init @@ -62,6 +62,9 @@ MODULE fp_api ! Lifecycle flags (module-scope state, single instance only at L-3). LOGICAL, SAVE :: g_initialized = .FALSE. LOGICAL, SAVE :: g_prepared = .FALSE. + ! Registry mutation counter seen by the last successful prep (#253). + ! -1 = never prepared. See fp_param_generation in the registry. + INTEGER, SAVE :: g_prep_generation = -1 CONTAINS @@ -135,10 +138,10 @@ FUNCTION fp_api_set_param(name, value) RESULT(ierr) BIND(C, NAME="fp_set_param") IF (fp_param_set(TRIM(fname), REAL(value, KIND=rkind)) /= 0) THEN ierr = FP_ERR_INVALID ELSE - ! A user who changes a profile-shape-altering variable needs to - ! re-run fp_prep; invalidate g_prepared so the next fp_run picks - ! up the new parameters. - g_prepared = .FALSE. + ! A successful set advances the registry's mutation counter; + ! fp_run compares it with g_prep_generation and re-runs fp_prep + ! (#253: the same holds for sets that reach the registry without + ! passing through here, such as tot's "fp:" dispatcher). ierr = FP_OK END IF END FUNCTION fp_api_set_param @@ -179,7 +182,8 @@ FUNCTION fp_api_set_param_str(name, value) RESULT(ierr) BIND(C, NAME="fp_set_par IF (fp_param_set_str(TRIM(fname), TRIM(fvalue)) /= 0) THEN ierr = FP_ERR_INVALID ELSE - g_prepared = .FALSE. + ! The registry's mutation counter makes the next fp_run re-prep + ! (see fp_api_set_param). ierr = FP_OK END IF END FUNCTION fp_api_set_param_str @@ -219,15 +223,18 @@ FUNCTION fp_api_run(ntmax_in) RESULT(ierr) BIND(C, NAME="fp_run") ! fp_prep allocates the FPCOMM arrays, sets up the velocity / radial ! mesh, and initializes the distribution function. It must run before ! the first fp_loop call and any time the caller updated a profile- - ! shape-altering namelist variable (fp_api_set_param clears - ! g_prepared). - IF (.NOT. g_prepared) THEN + ! shape-altering namelist variable: the registry counts every + ! successful set (fp_param_generation), from fp_api_set_param or + ! from tot's "fp:" dispatcher alike (#253), and a changed count + ! means re-prep. + IF (.NOT. g_prepared .OR. g_prep_generation /= fp_param_generation()) THEN CALL fp_prep(prep_ierr) IF (prep_ierr /= 0) THEN ierr = FP_ERR_CALC_FAILED RETURN END IF g_prepared = .TRUE. + g_prep_generation = fp_param_generation() END IF ! Override NTMAX for this call so fp_loop steps exactly ntmax_in @@ -301,7 +308,8 @@ FUNCTION fp_api_get_state(state) RESULT(ierr) BIND(C, NAME="fp_get_state") ! The arrays keep the extents of the last fp_prep (fp_allocate_ntg2 ! gives all six the same shape (NRMAX,NSAMAX,NTG2M)), while NRMAX / ! NSAMAX are the current namelist values. fp_set_param on a count - ! clears g_prepared without reallocating, so until the next fp_run + ! advances the registry counter without reallocating, so until the + ! next fp_run ! the two can disagree (#245). Copy MIN(current count, allocated ! extent) along each axis and report the copied extents, so the ! [nsamax][nrmax] slice the caller reads is exactly the data that diff --git a/fp/fp_param_registry.f90 b/fp/fp_param_registry.f90 index 95c7899ce..4b455d5c7 100644 --- a/fp/fp_param_registry.f90 +++ b/fp/fp_param_registry.f90 @@ -38,13 +38,47 @@ MODULE fp_param_registry IMPLICIT NONE PRIVATE PUBLIC :: fp_param_set + PUBLIC :: fp_param_generation PUBLIC :: fp_param_set_str PUBLIC :: fp_param_check PUBLIC :: parse_array_subscript_pub ! exported for unit-test only + INTEGER, SAVE :: g_generation = 0 + CONTAINS + !------------------------------------------------------------------- + ! Mutation counter (#253). + ! + ! Every successful set through this registry, whoever calls it + ! (fp_api_set_param, tot's "fp:" dispatcher, a future caller), + ! advances g_generation. fp_api compares the value it saw when it + ! last ran fp_prep with the current one and re-preps on a change, + ! so the "prepared" state cannot go stale behind the C ABI's back. + ! A set that fails leaves the counter alone: nothing changed. + !------------------------------------------------------------------- + FUNCTION fp_param_generation() RESULT(gen) + INTEGER :: gen + gen = g_generation + END FUNCTION fp_param_generation + FUNCTION fp_param_set(name, value) RESULT(ierr) + CHARACTER(LEN=*), INTENT(IN) :: name + REAL(rkind), INTENT(IN) :: value + INTEGER :: ierr + ierr = fp_param_set_impl(name, value) + IF (ierr == 0) g_generation = g_generation + 1 + END FUNCTION fp_param_set + + FUNCTION fp_param_set_str(name, value) RESULT(ierr) + CHARACTER(LEN=*), INTENT(IN) :: name + CHARACTER(LEN=*), INTENT(IN) :: value + INTEGER :: ierr + ierr = fp_param_set_str_impl(name, value) + IF (ierr == 0) g_generation = g_generation + 1 + END FUNCTION fp_param_set_str + + FUNCTION fp_param_set_impl(name, value) RESULT(ierr) CHARACTER(LEN=*), INTENT(IN) :: name REAL(rkind), INTENT(IN) :: value INTEGER :: ierr @@ -156,7 +190,7 @@ FUNCTION fp_param_set(name, value) RESULT(ierr) CASE DEFAULT ierr = 1 ! unknown name END SELECT - END FUNCTION fp_param_set + END FUNCTION fp_param_set_impl !------------------------------------------------------------------- ! fp_param_check : the counts a run cannot be made with, checked @@ -251,7 +285,7 @@ END FUNCTION fp_param_check ! KNAMEQ='eqdata' (from pl_init) is missing in cwd, so eq_load ! fails silently and downstream BESEKNX trips with NCALC=-2. !------------------------------------------------------------------- - FUNCTION fp_param_set_str(name, value) RESULT(ierr) + FUNCTION fp_param_set_str_impl(name, value) RESULT(ierr) CHARACTER(LEN=*), INTENT(IN) :: name CHARACTER(LEN=*), INTENT(IN) :: value INTEGER :: ierr @@ -262,7 +296,7 @@ FUNCTION fp_param_set_str(name, value) RESULT(ierr) CASE DEFAULT ierr = 1 ! unknown string-valued name END SELECT - END FUNCTION fp_param_set_str + END FUNCTION fp_param_set_str_impl !------------------------------------------------------------------- ! Split "NAME" or "NAME[N]" into (base, idx). diff --git a/python/fplib/tests/test_state_extents.py b/python/fplib/tests/test_state_extents.py index 625f1239a..3e1883dcc 100644 --- a/python/fplib/tests/test_state_extents.py +++ b/python/fplib/tests/test_state_extents.py @@ -2,8 +2,8 @@ The FPCOMM profile arrays (``RNT``, ``RWT``, ... in ``fp/fpcomm.f90``) keep the extents of the last ``fp_prep``; ``NRMAX`` / ``NSAMAX`` are the current -namelist values. ``fp_set_param`` on either count clears ``g_prepared`` -without reallocating, so until the next ``fp_run`` the two disagree, and +namelist values. ``fp_set_param`` on either count marks the module for +re-prep without reallocating, so until the next ``fp_run`` the two disagree, and ``fp_get_state`` used to copy ``nsa = 1..NSAMAX``, ``nr = 1..NRMAX`` out of arrays that are smaller than that (a read past the allocation: an abort under ``-fcheck=all``, garbage in the padding otherwise). diff --git a/python/totlib/README.md b/python/totlib/README.md index f3808ce68..3e65afcaf 100644 --- a/python/totlib/README.md +++ b/python/totlib/README.md @@ -126,6 +126,27 @@ Set one numeric parameter. `name` MUST be `":"`. Validation runs Python-side first, so missing/unknown prefixes raise `TotlibInvalidParamError` before any FFI call. +A set after a run restarts the module it addresses on that module's +next run: `tr:` / `fp:` / `ti:` parameters reach the module's registry, +which counts every successful set, and `tr_run` / `fp_run` / `ti_run` +re-run their `*_prep` (initial profiles, metric, BPSD state; `NT` and `T` +back to 0) when the count moved since their last prep. `Tot.run` drives +`tr_run` only (L-6), so for `tr:` that is the next `Tot.run`; for `fp:` / +`ti:` it is the next `fp_run` / `ti_run`, reached through `Fplib` / +`Tilib` on the mono image or through the C ABI. This is the same +semantics as `Trlib.set_param` / `TotPipeline.set_param` (#253). A set the +registry refuses (unknown name, bad index) changes nothing and the next +run continues. There is no "live" update of a running module through +`set_param`; a coupling input that should not restart the module is a +registry-level classification still to be designed. + +The count is per registry. On the mono image several modules read the +same `plcomm` variables (`ti:RR` and `eq:RR` are one `RR`), and a set +through one module's registry does not move another module's count: +after `Tot.set_param("ti:RR", ...)`, `fp_run` does not re-prep. That is +the pre-existing shared-storage property of the mono image, not something +`set_param` can see; set each module's own parameters. + ### `Tot.set_param_str(name, value) -> None` Set one string parameter. At L-3, only the `tr:` and `eq:` namespaces diff --git a/python/totlib/tests/test_set_after_run.py b/python/totlib/tests/test_set_after_run.py new file mode 100644 index 000000000..defbd87d9 --- /dev/null +++ b/python/totlib/tests/test_set_after_run.py @@ -0,0 +1,270 @@ +"""A parameter set after a run re-preps the module, whoever set it (#253). + +``tr_api`` / ``fp_api`` / ``ti_api`` used to clear their private "prepared" +flag inside ``_set_param`` only. tot's ``:`` dispatcher +(``tot/tot_param_registry.f90``) calls the registries directly, so a set +through ``Tot`` after a run left the flag standing: the next ``tot_run`` +continued the old state with ``tr_prep`` skipped, and the #189 pre-run +species check never fired. The registries now count every successful set +and the API re-preps when the count moved since its last prep. + +Observable: ``tr_prep`` resets ``NT`` and ``T`` to zero, so a set followed by +``run(1)`` leaves ``nt == 1`` (restart) instead of ``nt == previous + 1`` +(continue). That is what ``trlib`` has always done; ``Tot`` now agrees. + +fp and ti are checked on the mono image (``libtotapi_mono.so``, one copy +of every module) with raw ctypes: for fp a count changed through +``tot_set_param`` must show in the next ``fp_get_state`` (it reports the +allocated extent, #248), for ti ``NT`` must restart; both only happen when +``_prep`` ran again. The default image is not used for these two: +``libtotapi.so`` carries five PIC copies of pl / eq / bpsd (one per +sibling ``lib*api.so``, no ``-Bsymbolic``); on ELF the dynamic linker +picks one definer per symbol across all copies, so the pl / eq state +that ``tot_init`` prepared and the state ``fp_run`` touches are not +guaranteed to live in the same copy. ``fp_run`` through that image +crashed on Linux CI (SIGSEGV / SIGBUS) while the mono image passed; +macOS's two-level namespaces bind each library to its own copy and hide +it. No product path calls ``fp_run`` / ``ti_run`` through +``libtotapi.so`` after ``tot_init`` (``tot_run`` drives tr only; +``TotPipeline`` uses ``Fplib``'s own handle without ``tot_init``), and +running fp / ti alongside tot in one process is what the mono image +exists for. + +Process isolation: ``pytestmark`` requests per-test forking (effective when +pytest-forked is installed, which CI and local dev have), because every +module keeps SAVE'd state in the shared library. +""" +from __future__ import annotations + +import ctypes +import os +import sys +import unittest +from pathlib import Path + +import pytest + +pytestmark = [pytest.mark.forked] + +HERE = Path(__file__).resolve() +REPO = HERE.parents[3] +PYTHON_ROOT = REPO / "python" +if str(PYTHON_ROOT) not in sys.path: + sys.path.insert(0, str(PYTHON_ROOT)) + +from tilib.tests._data_cwd import TiDataCwdMixin # noqa: E402 + +DEFAULT_TOT_SO = REPO / "tot" / "libtotapi.so" +DEFAULT_TR_SO = REPO / "tr" / "libtrapi.so" + + +def _tot_so() -> Path: + env = os.environ.get("TOTLIB_PATH") + return Path(env) if env else DEFAULT_TOT_SO + + +def _tr_so() -> Path: + env = os.environ.get("TRLIB_PATH") + return Path(env) if env else DEFAULT_TR_SO + + +def _mono_so() -> str: + # Absolute before any chdir (TiDataCwdMixin moves into a temp dir). + p = os.environ.get("MONO_LIB_PATH", "") + return os.path.abspath(p) if p and os.path.exists(p) else "" + + +# The guards below run in setUp, not at class-decoration time: the .so +# files and MONO_LIB_PATH are mutable preconditions, and a decorator +# evaluates them once at import (Bugbot flagged that pattern on #221). +def _skip_unless_tot(tc: unittest.TestCase) -> None: + if not _tot_so().exists(): + tc.skipTest(f"libtotapi.so not built at {_tot_so()}; run `make -C tot libtotapi.so`") + + +def _skip_unless_tr(tc: unittest.TestCase) -> None: + if not _tr_so().exists(): + tc.skipTest(f"libtrapi.so not built at {_tr_so()}; run `make -C tr libtrapi.so`") + + +def _skip_unless_mono(tc: unittest.TestCase) -> None: + if not _mono_so(): + tc.skipTest("MONO_LIB_PATH not set or missing; build with " + "`make -C tot libtotapi_mono.so`") + + +# A small DT keeps each step cheap and finite on the default tr namelist +# (same choice as test_lifecycle.py). +_DT = 0.001 + + +class TestTotSetAfterRun(unittest.TestCase): + """``Tot.set_param("tr:...")`` after ``run`` restarts, as ``Trlib`` does.""" + + def setUp(self): + _skip_unless_tot(self) + _skip_unless_tr(self) + + def test_set_after_run_restarts_like_trlib(self): + from totlib import Tot + from trlib import Trlib + + with Tot() as tot: + tot.set_param("tr:DT", _DT) + tot.run(2) + tot_before = tot.get_state().nt + tot.set_param("tr:DT", 2 * _DT) + tot.run(1) + tot_after = tot.get_state().nt + + with Trlib() as tr: + tr.set_param("DT", _DT) + tr.run(2) + tr_before = tr.get_state().nt + tr.set_param("DT", 2 * _DT) + tr.run(1) + tr_after = tr.get_state().nt + + self.assertEqual((tr_before, tr_after), (2, 1), + "trlib: set_param after run must restart (tr_prep resets NT)") + self.assertEqual((tot_before, tot_after), (tr_before, tr_after), + "Tot must follow trlib's set-after-run semantics (#253)") + + def test_set_after_run_reaches_the_pre_run_check(self): + """The #189 species check runs where tr_prep is about to run; a + tot-dispatched NSMAX=3 after a run must reach it, not continue.""" + from totlib import Tot, TotlibError + + with Tot() as tot: + tot.set_param("tr:DT", _DT) + tot.run(1) + tot.set_param("tr:NSMAX", 3) # species 3 has PN = PNS = 0 + with self.assertRaises(TotlibError): + tot.run(1) + + def test_failed_set_does_not_restart(self): + """A set the registry refused changed nothing: the run continues.""" + from totlib import Tot, TotlibError + + with Tot() as tot: + tot.set_param("tr:DT", _DT) + tot.run(2) + with self.assertRaises(TotlibError): + tot.set_param("tr:NO_SUCH_PARAMETER", 1.0) + tot.run(1) + self.assertEqual(tot.get_state().nt, 3) + + +# --- fp / ti through the same image --------------------------------------- + +def _load_image(path: str) -> ctypes.CDLL: + lib = ctypes.CDLL(os.path.abspath(path)) + for name in ("tot_init", "tot_finalize"): + fn = getattr(lib, name) + fn.restype = ctypes.c_int + fn.argtypes = [] + lib.tot_set_param.restype = ctypes.c_int + lib.tot_set_param.argtypes = [ctypes.c_char_p, ctypes.c_double] + return lib + + +def _drive_through_tot(lib: ctypes.CDLL, mod: str, state_cls, *, + steps_before: int, param: str, value: float, + steps_after: int, field: str): + """tot_init; _run(steps_before); tot_set_param(":"); + _run(steps_after); tot_finalize. + + Returns ``field`` of ``_get_state`` before the set and after the + second run. + """ + run = getattr(lib, f"{mod}_run") + run.restype = ctypes.c_int + run.argtypes = [ctypes.c_int] + get_state = getattr(lib, f"{mod}_get_state") + get_state.restype = ctypes.c_int + get_state.argtypes = [ctypes.POINTER(state_cls)] + + rc = lib.tot_init() + if rc != 0: + raise AssertionError(f"tot_init returned {rc}") + try: + st = state_cls() + rc = run(steps_before) + if rc != 0: + raise AssertionError(f"{mod}_run({steps_before}) returned {rc} before the set") + get_state(ctypes.byref(st)) + before = getattr(st, field) + rc = lib.tot_set_param(f"{mod}:{param}".encode(), float(value)) + if rc != 0: + raise AssertionError(f"tot_set_param({mod}:{param}) returned {rc}") + rc = run(steps_after) + if rc != 0: + raise AssertionError(f"{mod}_run({steps_after}) returned {rc} after the set") + get_state(ctypes.byref(st)) + return before, getattr(st, field) + finally: + lib.tot_finalize() + + +class _FpReprepCase: + """fp body; the subclass says which image to load.""" + + image: str = "" + NEW_NRMAX = 20 + + def test_fp_reprepares_after_tot_set(self): + """fp_get_state reports the allocated radial extent (#245/#248), so + nrmax follows a changed NRMAX only once fp_prep reallocated.""" + from fplib._ffi import FpStateC + before, after = _drive_through_tot( + _load_image(self.image), "fp", FpStateC, + steps_before=0, param="NRMAX", value=self.NEW_NRMAX, + steps_after=0, field="nrmax") + self.assertNotEqual(before, self.NEW_NRMAX, + "pick a NEW_NRMAX that differs from fp's default") + self.assertEqual(after, self.NEW_NRMAX, + "fp_run after tot_set_param('fp:NRMAX') must re-run fp_prep (#253)") + + +class _TiReprepCase(TiDataCwdMixin): + """ti_prep reads ../adpost/ADPOST-DATA relative to the cwd; the mixin + from tilib's tests arranges that layout.""" + + image: str = "" + + def test_ti_reprepares_after_tot_set(self): + """ti_get_state reports NRMAX straight from the namelist, so the + observable is NT: ti_prep resets it to 0 (ti/tiprep.f90) and ti_run + advances it, so run(2), set, run(1) ends at 1 only after a re-prep + (3 when the old state continues).""" + from tilib._ffi import TiStateC + before, after = _drive_through_tot( + _load_image(self.image), "ti", TiStateC, + steps_before=2, param="NRMAX", value=20, + steps_after=1, field="nt") + self.assertEqual(before, 2, "ti_run(2) must advance NT to 2") + self.assertEqual(after, 1, + "ti_run after tot_set_param('ti:NRMAX') must re-run ti_prep (#253)") + + +class TestSiblingReprepOnMonoImage(_FpReprepCase, unittest.TestCase): + """libtotapi_mono.so: one image, one copy of every module's state. + + Runs in CI's mono routing step (MONO_LIB_PATH set); skipped elsewhere. + """ + + def setUp(self): + _skip_unless_mono(self) + self.image = _mono_so() + super().setUp() + + +class TestTiReprepOnMonoImage(_TiReprepCase, unittest.TestCase): + def setUp(self): + _skip_unless_mono(self) + self.image = _mono_so() + super().setUp() + + +if __name__ == "__main__": + unittest.main() diff --git a/ti/ti_api.f90 b/ti/ti_api.f90 index 103e57ae4..c6e0e9dac 100644 --- a/ti/ti_api.f90 +++ b/ti/ti_api.f90 @@ -8,8 +8,8 @@ ! populates TICOMM default values including NRMAX=50, ! NSMAX=2 (from pl_init), DT=0.01, NTSTEP=1, etc. ! ti_set_param -> dispatches to ti_param_registry::ti_param_set which -! handles ~35 namelist scalars/arrays. Clears g_prepared -! so profile-affecting changes pick up on the next run. +! handles ~35 namelist scalars/arrays. The registry's +! mutation counter makes the next ti_run re-prep (#253). ! ti_run -> ti_prep (first call only) + ti_exec with NTMAX set to ! the requested step count; NTMAX is restored after the ! loop. @@ -34,7 +34,7 @@ MODULE ti_api residual_loop_max, icount_loop_max, icount_mat_max, & RNA, RTA, RUA, RBP, RQP, RJP, ZEFF, BETA, BETAP, & allocate_ticomm, deallocate_ticomm - USE ti_param_registry, ONLY: ti_param_set + USE ti_param_registry, ONLY: ti_param_set, ti_param_generation USE plinit, ONLY: pl_init USE equnit, ONLY: eq_init USE tiinit, ONLY: tiinit_fortran => ti_init @@ -60,6 +60,9 @@ MODULE ti_api ! Lifecycle flags (module-scope state, single instance only at L-3). LOGICAL, SAVE :: g_initialized = .FALSE. LOGICAL, SAVE :: g_prepared = .FALSE. + ! Registry mutation counter seen by the last successful prep (#253). + ! -1 = never prepared. See ti_param_generation in the registry. + INTEGER, SAVE :: g_prep_generation = -1 CONTAINS @@ -149,10 +152,10 @@ FUNCTION ti_api_set_param(name, value) RESULT(ierr) BIND(C, NAME="ti_set_param") IF (ti_param_set(TRIM(fname), value) /= 0) THEN ierr = TI_ERR_INVALID ELSE - ! A user who changes a profile-shape-altering variable needs to - ! re-run ti_prep; invalidate g_prepared so the next ti_run picks - ! up the new parameters. - g_prepared = .FALSE. + ! A successful set advances the registry's mutation counter; + ! ti_run compares it with g_prep_generation and re-runs ti_prep + ! (#253: the same holds for sets that reach the registry without + ! passing through here, such as tot's "ti:" dispatcher). ierr = TI_OK END IF END FUNCTION ti_api_set_param @@ -180,15 +183,18 @@ FUNCTION ti_api_run(ntmax_in) RESULT(ierr) BIND(C, NAME="ti_run") ! ti_prep builds the initial profile, metric and bpsd state. It must ! run before the first ti_exec call and any time the caller updated - ! a profile-shape-altering namelist variable (ti_api_set_param - ! clears g_prepared). - IF (.NOT. g_prepared) THEN + ! a profile-shape-altering namelist variable: the registry counts + ! every successful set (ti_param_generation), from ti_api_set_param + ! or from tot's "ti:" dispatcher alike (#253), and a changed count + ! means re-prep. + IF (.NOT. g_prepared .OR. g_prep_generation /= ti_param_generation()) THEN CALL ti_prep(prep_ierr) IF (prep_ierr /= 0) THEN ierr = TI_ERR_CALC_FAILED RETURN END IF g_prepared = .TRUE. + g_prep_generation = ti_param_generation() END IF ! Override NTMAX for this call so ti_exec runs exactly ntmax_in diff --git a/ti/ti_param_registry.f90 b/ti/ti_param_registry.f90 index 446514994..4dda3f722 100644 --- a/ti/ti_param_registry.f90 +++ b/ti/ti_param_registry.f90 @@ -41,14 +41,40 @@ MODULE ti_param_registry IMPLICIT NONE PRIVATE PUBLIC :: ti_param_set + PUBLIC :: ti_param_generation INTEGER, PARAMETER :: ERR_OK = 0 INTEGER, PARAMETER :: ERR_UNKNOWN_NAME = 1 INTEGER, PARAMETER :: ERR_BAD_INDEX = 2 + INTEGER, SAVE :: g_generation = 0 + CONTAINS + !------------------------------------------------------------------- + ! Mutation counter (#253). + ! + ! Every successful set through this registry, whoever calls it + ! (ti_api_set_param, tot's "ti:" dispatcher, a future caller), + ! advances g_generation. ti_api compares the value it saw when it + ! last ran ti_prep with the current one and re-preps on a change, + ! so the "prepared" state cannot go stale behind the C ABI's back. + ! A set that fails leaves the counter alone: nothing changed. + !------------------------------------------------------------------- + FUNCTION ti_param_generation() RESULT(gen) + INTEGER :: gen + gen = g_generation + END FUNCTION ti_param_generation + FUNCTION ti_param_set(name, value) RESULT(ierr) + CHARACTER(LEN=*), INTENT(IN) :: name + REAL(rkind), INTENT(IN) :: value + INTEGER :: ierr + ierr = ti_param_set_impl(name, value) + IF (ierr == 0) g_generation = g_generation + 1 + END FUNCTION ti_param_set + + FUNCTION ti_param_set_impl(name, value) RESULT(ierr) CHARACTER(LEN=*), INTENT(IN) :: name REAL(rkind), INTENT(IN) :: value INTEGER :: ierr @@ -190,7 +216,7 @@ FUNCTION ti_param_set(name, value) RESULT(ierr) CASE DEFAULT ierr = ERR_UNKNOWN_NAME END SELECT - END FUNCTION ti_param_set + END FUNCTION ti_param_set_impl ! Parse "BASE", "BASE[i]", or "BASE[i,j]" into base + indices. ! i / j default to 0 when absent; -1 on parse error. diff --git a/tr/tr_api.f90 b/tr/tr_api.f90 index 05138b79d..6670cb7a0 100644 --- a/tr/tr_api.f90 +++ b/tr/tr_api.f90 @@ -40,7 +40,7 @@ MODULE tr_api AJRFT, & EXTERNAL_DRIVEN_I, EXTERNAL_DRIVEN_RW, & ALLOCATE_TRCOMM, DEALLOCATE_TRCOMM - USE tr_param_registry, ONLY: tr_param_set, tr_param_set_str + USE tr_param_registry, ONLY: tr_param_set, tr_param_set_str, tr_param_generation USE plinit, ONLY: pl_init USE equnit, ONLY: eq_init USE trinit, ONLY: trinit_fortran => tr_init @@ -71,6 +71,9 @@ MODULE tr_api LOGICAL, SAVE :: g_initialized = .FALSE. LOGICAL, SAVE :: g_prepared = .FALSE. + ! Registry mutation counter seen by the last successful prep (#253). + ! -1 = never prepared. See tr_param_generation in the registry. + INTEGER, SAVE :: g_prep_generation = -1 CONTAINS @@ -155,10 +158,10 @@ FUNCTION tr_api_set_param(name, value) RESULT(ierr) BIND(C, NAME="tr_set_param") IF (tr_param_set(TRIM(fname), value) /= 0) THEN ierr = TR_ERR_INVALID ELSE - ! A user who changes a profile-shape-altering variable needs to - ! re-run tr_prep; invalidate g_prepared so the next tr_run picks - ! up the new parameters. - g_prepared = .FALSE. + ! A successful set advances the registry's mutation counter; + ! tr_run compares it with g_prep_generation and re-runs tr_prep + ! (#253: the same holds for sets that reach the registry without + ! passing through here, such as tot's "tr:" dispatcher). ierr = TR_OK END IF END FUNCTION tr_api_set_param @@ -200,8 +203,8 @@ FUNCTION tr_api_set_param_str(name, value) RESULT(ierr) BIND(C, NAME="tr_set_par ierr = TR_ERR_INVALID ELSE ! Changing KNAMEQ (equilibrium data file) alters the tr_prep - ! inputs; invalidate g_prepared so the next tr_run rebuilds. - g_prepared = .FALSE. + ! inputs; the registry's mutation counter makes the next tr_run + ! rebuild (see tr_api_set_param). ierr = TR_OK END IF END FUNCTION tr_api_set_param_str @@ -232,10 +235,11 @@ FUNCTION tr_api_run(ntmax_in) RESULT(ierr) BIND(C, NAME="tr_run") END IF ! tr_prep builds the initial profile, metric and bpsd state. It must - ! run before the first tr_loop call and any time the caller updated - ! a profile-shape-altering namelist variable (tr_api_set_param - ! clears g_prepared). - IF (.NOT. g_prepared) THEN + ! run before the first tr_loop call and any time a parameter was set + ! since the last prep: the registry counts every successful set + ! (tr_param_generation), from tr_api_set_param or from tot's "tr:" + ! dispatcher alike (#253), and a changed count means re-prep. + IF (.NOT. g_prepared .OR. g_prep_generation /= tr_param_generation()) THEN ! NSMAX above the solver's compile-time NSM: tr_api_validate ! reports it, but a caller that skips validate must not reach ! tr_prep with it (the registry accepts up to TR_MAX_NSMAX=8). @@ -263,6 +267,7 @@ FUNCTION tr_api_run(ntmax_in) RESULT(ierr) BIND(C, NAME="tr_run") RETURN END IF g_prepared = .TRUE. + g_prep_generation = tr_param_generation() END IF ! Override NTMAX for this call so tr_loop steps exactly ntmax_in diff --git a/tr/tr_param_registry.f90 b/tr/tr_param_registry.f90 index 9612bce8c..4881e72fe 100644 --- a/tr/tr_param_registry.f90 +++ b/tr/tr_param_registry.f90 @@ -58,12 +58,46 @@ MODULE tr_param_registry IMPLICIT NONE PRIVATE PUBLIC :: tr_param_set + PUBLIC :: tr_param_generation PUBLIC :: tr_param_set_str PUBLIC :: parse_array_subscript_pub ! exported for unit-test only + INTEGER, SAVE :: g_generation = 0 + CONTAINS + !------------------------------------------------------------------- + ! Mutation counter (#253). + ! + ! Every successful set through this registry, whoever calls it + ! (tr_api_set_param, tot's "tr:" dispatcher, a future caller), + ! advances g_generation. tr_api compares the value it saw when it + ! last ran tr_prep with the current one and re-preps on a change, + ! so the "prepared" state cannot go stale behind the C ABI's back. + ! A set that fails leaves the counter alone: nothing changed. + !------------------------------------------------------------------- + FUNCTION tr_param_generation() RESULT(gen) + INTEGER :: gen + gen = g_generation + END FUNCTION tr_param_generation + FUNCTION tr_param_set(name, value) RESULT(ierr) + CHARACTER(LEN=*), INTENT(IN) :: name + REAL(rkind), INTENT(IN) :: value + INTEGER :: ierr + ierr = tr_param_set_impl(name, value) + IF (ierr == 0) g_generation = g_generation + 1 + END FUNCTION tr_param_set + + FUNCTION tr_param_set_str(name, value) RESULT(ierr) + CHARACTER(LEN=*), INTENT(IN) :: name + CHARACTER(LEN=*), INTENT(IN) :: value + INTEGER :: ierr + ierr = tr_param_set_str_impl(name, value) + IF (ierr == 0) g_generation = g_generation + 1 + END FUNCTION tr_param_set_str + + FUNCTION tr_param_set_impl(name, value) RESULT(ierr) CHARACTER(LEN=*), INTENT(IN) :: name REAL(rkind), INTENT(IN) :: value INTEGER :: ierr @@ -170,7 +204,7 @@ FUNCTION tr_param_set(name, value) RESULT(ierr) CASE DEFAULT ierr = 1 ! unknown name END SELECT - END FUNCTION tr_param_set + END FUNCTION tr_param_set_impl !------------------------------------------------------------------- ! tr_param_set_str : string-valued parameter setter. @@ -186,7 +220,7 @@ END FUNCTION tr_param_set ! KFNCVS, KUFDIR, KUFDEV, KUFDCG) can be added by extending the ! SELECT CASE below without touching the C ABI (only the name list). !------------------------------------------------------------------- - FUNCTION tr_param_set_str(name, value) RESULT(ierr) + FUNCTION tr_param_set_str_impl(name, value) RESULT(ierr) CHARACTER(LEN=*), INTENT(IN) :: name CHARACTER(LEN=*), INTENT(IN) :: value INTEGER :: ierr @@ -197,7 +231,7 @@ FUNCTION tr_param_set_str(name, value) RESULT(ierr) CASE DEFAULT ierr = 1 ! unknown string-valued name END SELECT - END FUNCTION tr_param_set_str + END FUNCTION tr_param_set_str_impl !------------------------------------------------------------------- ! Split "NAME" or "NAME[N]" into (base, idx).