Skip to content

Drop obsolete seed patch from Fortran build #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
neuromechanist merged 2 commits into from
Aug 23, 2026
Merged

Drop obsolete seed patch from Fortran build #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
120bf3f
Drop obsolete seed patch from Fortran build
neuromechanist Aug 23, 2026
41ee4c4
Update build docs for removed seed patch
neuromechanist Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .context/issue-84/phase1_build_notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Adds a `native-fortran-f64` backend to `benchmarks/benchmark_dimsweep.py` that t

## gfortran build portability (source targets ifort + MKL)

> Update (PR #295, 2026-08-23): item 2 below is obsolete -- #235 rewrote the source's seeding to the portable deterministic form, and build_amica.sh no longer patches it. Kept for history.

`amica15.f90` assumes Intel's toolchain. A plain `gfortran` + LAPACK build needs three
fixes, applied by `build_amica.sh` to a **build copy** (the tracked `pamica/amica15.f90`
reference is never modified):
Expand Down
13 changes: 7 additions & 6 deletions benchmarks/fortran/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,16 @@ one). Settings are matched to the other backends: `num_mix=3`, `pdftype=0`, `do_
## Build portability (gfortran, no MKL/AMD)

`amica15.f90` targets Intel's toolchain (`ifort` + MKL), so a plain `gfortran` + LAPACK
build needs three portability fixes. `build_amica.sh` applies them **without touching the
tracked reference source** (it patches a build copy under `build/src/`):
build needs two portability fixes. `build_amica.sh` applies them **without touching the
tracked reference source** (it compiles a build copy under `build/src/`):

1. **`-cpp`** so the source's `#ifdef MKL` guards resolve (MKL undefined -> the
`include 'mkl_vml.f90'` is skipped and the non-MKL branch is used).
2. **`random_seed` seed size** -- the source's size-2 seed array (sized for ifort) is
rejected by gfortran; the build copy uses a portable default seed. Only affects random
init, which was already clock-based (non-reproducible), so it is timing-neutral.
3. **`vmath_shim.c`** provides `vrda_exp`/`vrda_log` (the non-MKL branch calls AMD LibM's
(A third fix, a `random_seed` patch for the old ifort-only size-2 seed array, became
obsolete when #235 made the source seed portably and deterministically itself --
`random_seed(size=nseed)` + an allocated `seedvec`, sccn/amica PR #54; the build
script now only verifies that portable seeding is present.)
2. **`vmath_shim.c`** provides `vrda_exp`/`vrda_log` (the non-MKL branch calls AMD LibM's
vector exp/log) as libm loops, so no vendor math library is needed. IEEE-accurate; a
vendor SIMD math library could make the exp/log-heavy E-step somewhat faster, so treat
this as a portable-baseline timing, not a max-tuned one.
Expand Down
24 changes: 9 additions & 15 deletions benchmarks/fortran/build_amica.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,15 @@ work_src="$build_dir/src"
mkdir -p "$work_src"
cp "$src_dir/funmod2.f90" "$src_dir/amica15.f90" "$src_dir/amica15_header.f90" "$work_src/"

# Patch ONLY the build copy (the tracked amica15.f90 stays the read-only parity
# reference). gfortran's random_seed wants a PUT array of its own size (8);
# the source's size-2 seed array (sized for ifort/MKL) is rejected. That seed
# only affects random initialization, which is already clock-based (hence
# non-reproducible run-to-run), so a portable default seed is timing-neutral and
# result-equivalent for the benchmark.
sed -i.bak \
's/.*random_seed(PUT = c1 .*/ call random_seed() ! benchmark build: portable default seed (build_amica.sh)/' \
"$work_src/amica15.f90"
# sed exits 0 even on a no-op, so verify the patch landed: if the tracked source
# ever renames the seed line, an unpatched random_seed(PUT=...) would otherwise
# reach the compiler and (with -fallow-argument-mismatch) build a binary that
# only crashes at runtime.
if ! grep -q 'call random_seed() ! benchmark build' "$work_src/amica15.f90"; then
echo "ERROR: random_seed patch did not apply -- amica15.f90 source may have drifted" >&2
# No random_seed patch is needed anymore: since #235 the tracked source seeds
# portably itself (random_seed(size=nseed) then an allocated seedvec(nseed),
# sccn/amica PR #54), which gfortran accepts and which keeps the deterministic
# `seed`-param behavior the parity tests rely on. The old sed patch targeted the
# pre-#235 size-2 ifort-only seed line and would now both fail its anchor check
# and, if it matched, destroy the deterministic seeding. Verify the portable
# seeding is present so a future source change fails loudly here, not at runtime.
if ! grep -q 'call random_seed(size = nseed)' "$work_src/amica15.f90"; then
echo "ERROR: expected portable random_seed(size=nseed) seeding in amica15.f90 (post-#235); source drifted" >&2
exit 1
fi

Expand Down
Loading