Files
TRIMSP/ToDo.txt
T
Zaher Salman 04c7c6b144 Record follow-ups from the parIRL / parERC fix
Adds a ToDo section for the three things deliberately left out of 93f0b1c, so
they are not lost:

- startSim() switches to the Results tab and purges the plots before
  runSimulationSequence() validates, so every refused run lands on an empty
  Results tab. Predates this work and affects all abort paths equally.
- scripts/deploy-web.sh only rewrites TRIMSP_BIN in the CGI and does not build,
  so the engine-side fix needs a rebuild and install before it reaches the live
  site. The GUI refusal works immediately, so a deployment can briefly pair the
  friendly GUI message with an engine that still segfaults on a hand-written
  deck.
- The untracked scratch in the tree (SrTiO3_E2000.inp, fortran/Makefile_org,
  fortran/profile*.txt, tests/{pcg32,ranlux,xoshiro}-test/) needs a decision on
  gitignore versus delete; .gitignore covers gmon.out and
  fortran/trimspNL-profile but not these.
2026-09-26 19:06:51 +02:00

151 lines
9.1 KiB
Plaintext

Seed handling and input hardening (deferred from the B-list fixes)
=====================================================================
DONE - Validate ranSeed in the GUI and startSequence: it must be an integer in a
safe range (|v| <= 2^31-1, matching ISEED = IDINT(RI) in trimspNL.F). Enforced
in runSimulationSequence() (before any job is generated) AND in CreateInpFile()
(defense-in-depth), so a crafted share link or direct deck builder is blocked
with a "please change it" message. Non-numeric / NaN / non-integer / oversized
seeds no longer reach the engine.
DONE - Derive per-scan-point seeds deterministically from the base seed and the
scan index, so a multi-point run is reproducible in any finish order
(currently: initialSeed + iScan, only when the user set a valid seed).
DONE - Cap valEnergy magnitude at 60000 eV (positive incidence mode and the
Maxwellian transmission mode |E0| = temperature). Stopping fits (KDEE1=4) are
documented valid to ~50 keV for helium (meaning-of-params.txt:177-185), so 60
keV is the model's fitted ceiling. Applied to scanned energies and the base
value in runSimulationSequence() and CreateInpFile().
- Warn when a shared configuration fixed the RanSeed: every scan point then uses
the same generator, which can produce statistically misleading results.
DONE (advisory, not a hard cap) - Warn on the web when numberProj exceeds 25000
per point. Measured cost model on a 3.2 GHz core, single-threaded engine:
runtime is linear in numberProj and linear in energy, and cost per projectile
is dominated by the number of nuclear collisions along the track, so the
expensive axis is the LIGHT projectiles, not the heavy ones. Projectile ->
Si, Maxwellian 60 keV, ZBL potential, ms/projectile: muon 3.04, H 3.30,
He 4.06 (worst), Li-8 3.91, Be-11 3.64, B-12 3.19, Mg-31 1.34, Ar 0.83.
Checked and found NOT to affect runtime: layer count (1..60 layers is flat,
the species lookup is a binary search), depth increment dz (0.1 A == 20 A),
and grazing incidence (ALPHA 89 deg is cheaper - the ion skims out).
NH=25000 at the worst case measured 98.6/99.5/100.1 s on the 1.3.2 binary and
101.5/100.7/101.7 s on a fresh 1.4.0 build, i.e. ~3x margin under the 300 s
TRIMSP_CMD_TIMEOUT. Deliberately warning-only: the value is advisory, the run
still starts, and the user keeps the final say. Web-only, because the
standalone app has no watchdog. TRIMSP_WEB_PROJ_ADVISORY in TrimSPlib.js;
uses the max of the sweep when scanning numberProj. The dead GUI
ProjNumberLimit() (TrimSPlib.js:3516) was left alone - it clamps at 5000,
which is below the 50000-100000 used by the publication test configs, so
wiring it up as-is would have clamped the regression suite.
- Persisting the effective per-point seed: the seed already lives in each
archived *.inp (all three deck slots), so reproducibility does not require the
*.seq header change; extending *.seq would shift the 999(A20,1x) layer slots
and break getSequenceColumnInfo(). Surfacing it in run details instead is
deferred - not sure it is needed.
Out-of-bounds recoil write in the sputtering path (parIRL >= 1)
=============================================================
FIXED - the engine now refuses the divergent input instead of corrupting memory.
Symptom, as originally reported: setting parIRL (the recoil/sputter output
level) to 1 crashed the engine with SIGSEGV, reproducibly 5/5 seeds, and end
to end through singleTrimSP.cgi as exitCode 139 / ok:false. parIRL=0 was never
affected.
Root cause was NOT the KO dimension. The first diagnosis blamed
KO(600,MAXNL5,2) at trimspNL.F:114 (debug: "Index '601' of dimension 1 of
array 'ko' above upper bound of 600"), on the grounds that the other
recoil-list arrays are 2000 wide. Enlarging KO to 2000 does not help: with
KO(2000) the run still dies, because the cascade overruns all of them.
The actual cause is a non-converging cascade driven by parERC = 0. The
primary knock-on test is `IF(DEN(IV).LE.ERC) GO TO 6`, so with a zero recoil
cutoff every atom that receives any energy at all is respawned as a recoil on
every cascade generation. The branching ratio exceeds one, so NREC1 and NREC2
grow geometrically instead of draining:
NREC2 = 81, 153, 269, 455, 773, 1317, ... (parERC=0, parIRL=1)
and the list-2/list-1 loop at label 83 never terminates. Instrumented runs
show the block is never even entered when parIRL=0. parERC defaults to 0.0
in the GUI (TrimSP.html:293) and tests/Si_1layer/Si_1layer_E19000.inp carries
0.0, so the failing combination is the default one - no unusual user action
is required to hit it.
Evidence that a real cutoff converges: forcing parERC to 0.02, 0.05 or 0.2
makes parIRL=1 finish normally (exit 0), and the peak cascade depth is only
103, well inside KO's 600. KO was therefore left at 600, and the resize was
reverted (it cost 7.7 MiB of .lbss and fixed nothing).
Changes in fortran/trimspNL.F:
- After readGuiInput, reject parIRL>=1 with parERC<=0 using a clear message
and STOP 1, before any transport work.
- The `IF(NREC2.GT.2000)` guard now stops with a non-zero status instead of
falling through into the out-of-bounds write loop, so any future
non-convergent cascade fails cleanly rather than corrupting memory.
The GUI now catches this before the engine is ever invoked:
- handleRecoilModeChange (wired to the parIRL widget) fills parERC with
TRIMSP_MIN_RECOIL_CUTOFF = 0.02 when recoil calculation is switched on with
a cutoff of 0, and says so in a transient warning.
- A pre-run check in runSimulationSequence refuses the run if the user then
insists on parIRL=1 with parERC=0, reports it in the status bar plus a
transient warning, and focuses the parERC field. The handler is deliberately
not wired to parERC, so retyping the cutoff is what "insisting" means and is
not silently overridden. This also covers shared-configuration links, which
assign widget values directly and so bypass the change handler.
- The parIRL tooltip documents the requirement.
Verified: parERC=0/parIRL=1 -> exit 1 with the new message; parERC=0.02 with
parIRL=0 or 1 -> exit 0; parERC=0 with parIRL=0 -> exit 0 and byte-identical
.rge (and an identical .out apart from timestamps/elapsed/filenames) versus
the pre-fix binary. tests/cgi-tests.sh 18/18, plus
tests/recoil-cutoff-test.js (12 cases over the shipped cutoff code) for the
GUI behaviour above.
Minimal repro: tests/Si_1layer/Si_1layer_E19000.inp with the last field of
line 2 changed from 0 to 1 (parERC is field 10 of line 1, still 0.0).
Deferred follow-ups from the C-list CGI hardening
=================================================
- The perl "Statement unlikely to be reached" warning at the exec/_exit(127)
child branch of run_bounded_command is intentional (matching muSRFlow) and
does not affect `perl -c`; tests run with -X to keep output clean.
- No global concurrency cap is implemented (by design, mirroring muSRFlow):
concurrent per-point engine runs are allowed; archive is serialized per run
with flock. Revisit if a runaway share link floods scan points.
- Auth (MUCH-need, muSRFlow parity): none is implemented, keeping the read-only
/tmp contract. If this CGI is ever exposed beyond a trusted LAN, gate it with
httpd authentication or a shared-token header.
- Restart integration (retch): the watchdog returns exit code 124 and the JSON
"log" carries the description; a deployment that wants automated retries can
key off exitCode === 124 and re-post the same .inp.
- A future client change could surface TRIMSP_TMPROOT behind a shared
configuration; the browser path regex is derived from the same env var, so
both sides stay consistent.
Follow-ups from the parIRL / parERC fix
=======================================
- startSim() switches to the Results tab and purges the plots BEFORE
runSimulationSequence() validates anything, so every refused run drops the
user on an empty Results tab with no explanation of why. This predates the
parIRL work and affects all the abort paths equally (energy cap, seed
validation, recoil cutoff, and the re-entrancy guard in startSequence). Fix
by validating in startSim() before the tab switch, or by moving the tab
switch after the first successful job is queued.
- The web deployment runs TRIMSP_BIN, which defaults to /usr/local/bin/trimspNL,
so the engine-side fix in trimspNL.F does not reach the live site until the
binary is rebuilt and installed. scripts/deploy-web.sh only rewrites the path
in the CGI; it does not build. The GUI-side fix works immediately, so a
deployment can briefly have the friendly GUI refusal in front of an engine
that still segfaults on a hand-written deck.
- Untracked scratch left in the tree, decide .gitignore vs delete:
SrTiO3_E2000.inp (2 keV / 1000-projectile benchmark deck at the repo root),
fortran/Makefile_org (Makefile backup), fortran/profile.txt and
fortran/profile_annotated.txt (gprof reports; .gitignore covers gmon.out and
fortran/trimspNL-profile but not these), and tests/pcg32-test/,
tests/ranlux-test/, tests/xoshiro-test/ (per-backend engine run output trees
from the RNG comparison, i.e. generated results rather than fixtures).