Skip to content

Commit 982ec89

Browse files
committed
fp-stability: remove Tier 2 per-instance disambiguation entirely
Per the weight review, the precision-build-based per-instance disambiguation was the heaviest piece (its own module + a build flag + CMake plumbing + tests) for the narrowest trigger (fires only when the most-flagrant hotspot is also inside a #:for/#:def expansion). Removed in full: - deleted toolchain/mfc/fp_precision_lines.py and its tests; deleted _disambiguate_instances - reverted CMakeLists.txt and build.py to upstream (no MFC_FP_PRECISION_LINES option, no marker-strip step, no -D flag); dropped the --fp-precision-lines build arg and the --precision-sim-binary fp-stability arg - removed the E3 disambiguation stage, its docstring section, and the per-instance summary display Kept: the lightweight '#:for/#:def-expanded — may represent multiple instances' hotspot warning (cheap, honest, separate from the disambiguation machinery). 57 toolchain tests, ruff, precheck all 7 green; CMakeLists.txt and build.py are byte-identical to upstream.
1 parent 9f868c7 commit 982ec89

8 files changed

Lines changed: 2 additions & 372 deletions

File tree

CMakeLists.txt

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ option(MFC_DOCUMENTATION "Build documentation" OFF
3131
option(MFC_ALL "Build everything" OFF)
3232
option(MFC_SINGLE_PRECISION "Build single precision" OFF)
3333
option(MFC_MIXED_PRECISION "Build mixed precision" OFF)
34-
option(MFC_FP_PRECISION_LINES "Strip fypp markers for per-instance fp-stability attribution" OFF)
3534

3635
if (MFC_ALL)
3736
set(MFC_PRE_PROCESS ON FORCE)
@@ -434,24 +433,8 @@ macro(HANDLE_SOURCES target useCommon)
434433
cmake_path(GET fpp FILENAME fpp_filename)
435434
set(f90 "${CMAKE_BINARY_DIR}/fypp/${target}/${fpp_filename}.f90")
436435

437-
# In a precision-lines build, Fypp writes a marked intermediate that is
438-
# then stripped of its line markers (so each expanded instance compiles
439-
# to a distinct physical line) before compilation; the strip step emits a
440-
# .linemap.json sidecar. Otherwise Fypp writes ${f90} directly. Only the
441-
# simulation target is analyzed by fp-stability, so pre/post_process are
442-
# always built normally.
443-
set(_precision_lines OFF)
444-
if (MFC_FP_PRECISION_LINES AND "${target}" STREQUAL "simulation")
445-
set(_precision_lines ON)
446-
endif()
447-
if (_precision_lines)
448-
set(f90_out "${CMAKE_BINARY_DIR}/fypp/${target}/${fpp_filename}.marked.f90")
449-
else()
450-
set(f90_out "${f90}")
451-
endif()
452-
453436
add_custom_command(
454-
OUTPUT ${f90_out}
437+
OUTPUT ${f90}
455438
COMMAND ${FYPP_EXE} -m re
456439
-I "${CMAKE_BINARY_DIR}/include/${target}"
457440
-I "${${target}_DIR}/include"
@@ -467,25 +450,12 @@ macro(HANDLE_SOURCES target useCommon)
467450
--line-length=999
468451
--line-numbering-mode=nocontlines
469452
${FYPP_GCOV_OPTS}
470-
"${fpp}" "${f90_out}"
453+
"${fpp}" "${f90}"
471454
DEPENDS "${fpp};${${target}_incs}"
472455
COMMENT "Preprocessing (Fypp) ${fpp_filename}"
473456
VERBATIM
474457
)
475458

476-
if (_precision_lines)
477-
add_custom_command(
478-
OUTPUT ${f90}
479-
COMMAND ${Python3_EXECUTABLE}
480-
"${CMAKE_SOURCE_DIR}/toolchain/mfc/fp_precision_lines.py"
481-
"${f90_out}" "${f90}"
482-
"${CMAKE_BINARY_DIR}/fypp/${target}/${fpp_filename}.linemap.json"
483-
DEPENDS "${f90_out};${CMAKE_SOURCE_DIR}/toolchain/mfc/fp_precision_lines.py"
484-
COMMENT "Stripping markers (fp-precision-lines) ${fpp_filename}"
485-
VERBATIM
486-
)
487-
endif()
488-
489459
list(APPEND ${target}_SRCs ${f90})
490460
endforeach()
491461
endmacro()

toolchain/mfc/build.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,6 @@ def configure(self, case: Case):
421421
flags.append(f"-DMFC_GCov={'ON' if ARG('gcov') else 'OFF'}")
422422
flags.append(f"-DMFC_Unified={'ON' if ARG('unified') else 'OFF'}")
423423
flags.append(f"-DMFC_Fastmath={'ON' if ARG('fastmath') else 'OFF'}")
424-
flags.append(f"-DMFC_FP_PRECISION_LINES={'ON' if ARG('fp_precision_lines') else 'OFF'}")
425424

426425
command = ["cmake"] + flags + ["-S", cmake_dirpath, "-B", build_dirpath]
427426

toolchain/mfc/cli/commands.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,6 @@
141141
default=False,
142142
dest="deps_only",
143143
),
144-
Argument(
145-
name="fp-precision-lines",
146-
help="(fp-stability) Strip fypp line markers so each expanded instance gets a distinct line; emits sidecars for per-instance attribution.",
147-
action=ArgAction.STORE_TRUE,
148-
default=False,
149-
dest="fp_precision_lines",
150-
),
151144
],
152145
examples=[
153146
Example("./mfc.sh build", "Build all default targets (CPU)"),
@@ -945,13 +938,6 @@
945938
default=None,
946939
metavar="PATH",
947940
),
948-
Argument(
949-
name="precision-sim-binary",
950-
help="Path to a simulation binary built with --fp-precision-lines. When given, macro-ambiguous hotspots are disambiguated to the individual fypp-expanded instance.",
951-
default=None,
952-
dest="precision_sim_binary",
953-
metavar="PATH",
954-
),
955941
Argument(
956942
name="samples",
957943
short="N",

toolchain/mfc/fp_precision_lines.py

Lines changed: 0 additions & 123 deletions
This file was deleted.

toolchain/mfc/fp_stability.py

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,6 @@
4848
One run with --check-max-float=yes; reports locations where a
4949
double→float conversion would overflow to ±Inf.
5050
51-
I. Per-instance disambiguation (--precision-sim-binary PATH; opt-in)
52-
A fypp #:for/#:def expansion collapses many generated computations onto one
53-
.fpp line, so a macro-ambiguous hotspot cannot be pinned to a single runtime
54-
instance. Given a simulation binary built with `--fp-precision-lines` (markers
55-
stripped so each instance is a distinct line, plus .linemap.json sidecars), the
56-
most flagrant macro-ambiguous hotspot is disambiguated: each expanded instance
57-
is perturbed alone on the precision binary, ranking them to the responsible
58-
instance and showing its concrete generated code.
59-
6051
Logs are saved to fp-stability-logs/ and uploaded as CI artifacts.
6152
On GitHub Actions: a step summary table and ::warning:: file annotations
6253
are emitted automatically so failing source lines appear in the PR diff.
@@ -95,7 +86,6 @@
9586
_emit_github_summary,
9687
)
9788
from .fp_stability_runners import (
98-
_disambiguate_instances,
9989
_find_binary,
10090
_find_verrou,
10191
_run_cancellation_check,
@@ -419,7 +409,6 @@ def _run_case(
419409
run_cancellation: bool,
420410
run_mca: bool,
421411
run_float_max: bool,
422-
prec_sim_bin: str = None,
423412
) -> dict:
424413
name = case["name"]
425414
compare = case["compare"]
@@ -542,24 +531,6 @@ def _run_case(
542531
except Exception as exc:
543532
cons.print(f" [bold yellow]dd_line confirmation error[/bold yellow]: {exc}")
544533

545-
# --- E3: per-instance disambiguation of the most flagrant macro-ambiguous hotspot ---
546-
if prec_sim_bin and result["dd_line_locs"]:
547-
macro_loc = next((loc for loc in result["dd_line_locs"] if loc.get("macro")), None)
548-
if macro_loc:
549-
cons.print(f" [dim]disambiguating fypp instances of {macro_loc['path']}:{macro_loc['start']} (precision binary)...[/dim]")
550-
try:
551-
insts = _disambiguate_instances(case, prec_sim_bin, verrou_bin, work_dir, macro_loc["path"], macro_loc["start"])
552-
macro_loc["instances"] = insts
553-
if insts and insts[0]["dev"] > 0:
554-
win = insts[0]
555-
cons.print(f" flagrant instance: #{win['instance']} (.f90:{win['physline']}, dev={win['dev']:.3e}) {win['snippet']}")
556-
elif insts:
557-
cons.print(f" [dim]{len(insts)} instance(s) enumerated; none perturbed measurably (hotspot inert)[/dim]")
558-
else:
559-
cons.print(" [dim]no sidecar instances found for this hotspot[/dim]")
560-
except Exception as exc:
561-
cons.print(f" [bold yellow]instance disambiguation error[/bold yellow]: {exc}")
562-
563534
# --- F: cancellation detection ---
564535
if run_cancellation:
565536
cons.print(" [dim]cancellation detection...[/dim]")
@@ -638,9 +609,6 @@ def fp_stability():
638609
run_cancellation = not ARG("no_cancellation")
639610
run_mca = not ARG("no_mca")
640611
run_float_max = not ARG("no_float_max")
641-
prec_sim_bin = ARG("precision_sim_binary")
642-
if prec_sim_bin and not os.path.isfile(prec_sim_bin):
643-
raise MFCException(f"precision simulation binary not found: {prec_sim_bin}")
644612

645613
log_dir = os.path.join(MFC_ROOT_DIR, "fp-stability-logs")
646614
os.makedirs(log_dir, exist_ok=True)
@@ -650,8 +618,6 @@ def fp_stability():
650618
cons.print(f" verrou: {verrou_bin}")
651619
cons.print(f" simulation: {sim_bin}")
652620
cons.print(f" pre_process: {pp_bin}")
653-
if prec_sim_bin:
654-
cons.print(f" precision: {prec_sim_bin} (per-instance disambiguation)")
655621
cons.print(f" samples: {n_samples}")
656622
features = []
657623
if run_float:
@@ -690,7 +656,6 @@ def fp_stability():
690656
run_cancellation,
691657
run_mca,
692658
run_float_max,
693-
prec_sim_bin,
694659
)
695660
except MFCException as exc:
696661
cons.print(f" [bold red]ERROR[/bold red]: {exc}")

toolchain/mfc/fp_stability_report.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,6 @@ def _emit_github_summary(results: list, n_samples: int):
201201
tags.append(f"_{loc['macro']}-expanded, may represent multiple instances_")
202202
suffix = f" — {', '.join(tags)}" if tags else ""
203203
md.append(f"- `{where}`{suffix}")
204-
for inst in loc.get("instances", [])[:8]:
205-
flag = " ⟵ flagrant" if inst is loc["instances"][0] and inst["dev"] > 0 else ""
206-
md.append(f" - instance #{inst['instance']} (`.f90:{inst['physline']}`, dev={inst['dev']:.2e}){flag}: `{inst['snippet']}`")
207204
snippet = _get_source_context(rel_path, start)
208205
if snippet:
209206
md.append(" ```fortran")

toolchain/mfc/fp_stability_runners.py

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
_parse_rddmin_syms,
2929
_parse_vg_error_locs,
3030
_rank_locs,
31-
_read_source_line,
3231
)
3332
from .printer import cons
3433

@@ -477,54 +476,3 @@ def _run_confirmation(case, verrou_bin, sim_bin, work_dir, ref_dir, dd_line_locs
477476
loc["share_dev"] = _source_perturb_dev(verrou_bin, sim_bin, work_dir, ref_dir, conf_dir, one, compare, f"line{i:02d}") if one else 0.0
478477
ranked = _rank_locs(dd_line_locs, total=(float_proxy or set_dev))
479478
return confirmed, set_dev, ranked
480-
481-
482-
def _disambiguate_instances(case, prec_sim_bin, verrou_bin, work_dir, hotspot_file, hotspot_line):
483-
"""Rank the individual fypp-expanded instances of a macro-ambiguous hotspot.
484-
485-
Uses a precision binary (built with --fp-precision-lines) in which each
486-
expanded instance of hotspot_file:hotspot_line compiles to a distinct
487-
physical .f90 line. The sidecar enumerates those physical lines; each is
488-
perturbed alone (float mode, vs the precision binary's own nearest-rounding
489-
reference) so the dominant instance is identified.
490-
491-
Returns a list of {instance, physline, dev, snippet} sorted most-flagrant
492-
first (empty if no sidecar / no instrumented instances).
493-
"""
494-
from . import fp_precision_lines as fpl
495-
496-
sidecar_dir = fpl.sidecar_dir_for_binary(prec_sim_bin)
497-
sidecar = fpl.load_sidecar(fpl.sidecar_path(sidecar_dir, hotspot_file))
498-
instances = fpl.instances_of(sidecar, hotspot_file, hotspot_line)
499-
if not instances:
500-
return []
501-
502-
prec_dir = os.path.join(work_dir, "precision")
503-
ref_dir = os.path.join(prec_dir, "ref")
504-
os.makedirs(ref_dir, exist_ok=True)
505-
try:
506-
_run_simulation_verrou(verrou_bin, prec_sim_bin, work_dir, ref_dir, rounding_mode="nearest")
507-
except MFCException:
508-
return []
509-
gen_lines = _capture_gen_source(verrou_bin, prec_sim_bin, work_dir, prec_dir, os.path.join(prec_dir, "gen_source.txt"))
510-
if gen_lines is None:
511-
return []
512-
513-
f90_file = os.path.join(sidecar_dir, os.path.basename(hotspot_file) + ".f90")
514-
compare = case["compare"]
515-
results = []
516-
for physline, instance in instances:
517-
src = _build_source_filter(gen_lines, [(f90_file, physline, physline)])
518-
if not src:
519-
continue # this instance performs no instrumented FP op
520-
dev = _source_perturb_dev(verrou_bin, prec_sim_bin, work_dir, ref_dir, prec_dir, src, compare, f"inst{instance:02d}")
521-
results.append(
522-
{
523-
"instance": instance,
524-
"physline": physline,
525-
"dev": dev or 0.0,
526-
"snippet": _read_source_line(f90_file, physline).strip(),
527-
}
528-
)
529-
results.sort(key=lambda r: r["dev"], reverse=True)
530-
return results

0 commit comments

Comments
 (0)