Skip to content

Register direct Fortio gate and align pins - #118

Merged
GeorgGrassler merged 11 commits into
mainfrom
agent/fortio-direct-gate
Aug 31, 2026
Merged

Register direct Fortio gate and align pins#118
GeorgGrassler merged 11 commits into
mainfrom
agent/fortio-direct-gate

Conversation

@krystophny

@krystophny krystophny commented Aug 19, 2026

Copy link
Copy Markdown
Member

What changed

  • register fortio_ref on the fast test workflow and the golden workflow’s explicit/manual dispatch, forwarding both dependency refs through Make and CMake
  • pin direct Fortio to 829fcde02024047f05e7a2389a01709617525e13 (continuation-reader and bulk complex-writer fix)
  • pin the default libneo dependency to 95f5c877d8f6185453f74b1e74b5ea3b17a20e7c, whose complete build metadata pins the same Fortio commit
  • quote benchmark result paths flagged by actionlint

Why

rabe directly consumes Fortio as well as consuming it transitively through libneo. The upstream registry audit found that the direct edge could not receive a candidate Fortio commit, so upstream compatibility testing was incomplete; fortio_ref now reaches that edge.

Verification

  • actionlint passes across all workflows
  • fresh candidate configure/build passes with explicit Fortio and libneo SHAs
  • GitHub test, golden, benchmark, and docs checks are green for the prior pin update; the current pin update is running the same gates
  • the pre-existing Fourier-field numerical-oracle mismatch is unchanged by this dependency-only patch

Gate design

test.yml is the registered fast Fortio compatibility gate. golden.yml is the slower strict reference suite, and benchmark.yml remains an independent performance comparison.

@krystophny
krystophny enabled auto-merge (squash) August 19, 2026 06:31

@GeorgGrassler GeorgGrassler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krystophny content wise fine, but I need some more context:

(a) the summary says that only 35/36 tests passed - considering the green gates I assume that this is a wrong/outdated statement, correct?
(b) explain to me again what triggered this pr: fortio was already fetched explicitly by rabe and did not rely on libneo for that. In principal the addition of a *_ref overwrite in line with the other dependencies, but I wanna understand what caused the addition now and how it fixes it.
(c) why are you only enable overwrite fortio_ref for test.yml and not e.g. golden.yml where also a libneo_ref overwrite happens? Again, not necessary a change request, just wanna understand.

Comment thread cmake/fortio.cmake Outdated
@@ -1,10 +1,13 @@
include(FetchContent)

set(FORTIO_REF "471256b7382afb8f641f37d833cc9172605da018" CACHE STRING

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this pattern - apply the same to $(LIBNEO_REF) for consistency

@krystophny krystophny Aug 19, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re right—I had the direction reversed. Corrected in ab9611b: Fortio keeps its direct default-pin pattern, and rabe’s LIBNEO_REF now matches it. No libneo repository change is needed.

@GeorgGrassler

Copy link
Copy Markdown
Collaborator

I will approve once questions are answered (and applicable changes are made).

@krystophny

Copy link
Copy Markdown
Member Author

Thanks — brief answers:

(a) Yes, the 35/36 figure was from the earlier local full-suite summary and is stale for this PR. The current test, golden, and benchmark gates are green; the one local miss was the pre-existing TestFourierFieldAccuracy oracle mismatch.

(b) Correct: rabe already fetches Fortio directly. This PR adds fortio_ref so the libneo downstream gate can inject the Fortio candidate SHA into that existing fetch; otherwise the gate would test rabe against its normal Fortio pin.

(c) test.yml is the registered direct Fortio compatibility gate. golden.yml and benchmark.yml remain their independent reference/benchmark suites; they accept libneo_ref where applicable but are not the direct Fortio candidate gate.

No additional code change is required for these points.

@krystophny

krystophny commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Correction: I had reversed the requested direction. Final fix is ab9611b: Fortio keeps its direct default-pin pattern, and rabe’s LIBNEO_REF now matches it. No libneo repository change is needed.

@GeorgGrassler

GeorgGrassler commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Thanks — brief answers:

(a) Yes, the 35/36 figure was from the earlier local full-suite summary and is stale for this PR. The current test, golden, and benchmark gates are green; the one local miss was the pre-existing TestFourierFieldAccuracy oracle mismatch.

(b) Correct: rabe already fetches Fortio directly. This PR adds fortio_ref so the libneo downstream gate can inject the Fortio candidate SHA into that existing fetch; otherwise the gate would test rabe against its normal Fortio pin.

(c) test.yml is the registered direct Fortio compatibility gate. golden.yml and benchmark.yml remain their independent reference/benchmark suites; they accept libneo_ref where applicable but are not the direct Fortio candidate gate.

No additional code change is required for these points.

but why is golden.yml not a Fortio candidate gate? Too long runtime?
@krystophny

@krystophny
krystophny force-pushed the agent/fortio-direct-gate branch from d64648c to 8dc510a Compare August 24, 2026 20:19
@krystophny

Copy link
Copy Markdown
Member Author

@GeorgGrassler Thanks — I addressed the review points and clarified the distinction in the PR description.

(a) The 35/36 figure was the local quick-suite result, not the current GitHub gate verdict. The test, golden, benchmark, and docs checks are green on the current head. The one local miss is the pre-existing TestFourierFieldAccuracy grid-65 numerical-oracle mismatch; this PR does not touch that code or oracle.

(b) You were right that rabe already fetches Fortio directly. The missing piece was candidate injection: without FORTIO_REF, an upstream Fortio downstream run would still build rabe against rabe’s normal Fortio pin. The PR now carries fortio_ref through workflow dispatch → Make → -DFORTIO_REF → the existing direct FetchContent edge. LIBNEO_REF uses the same conditional override pattern, with no libneo repository change.

(c) The automatic direct compatibility target is test.yml: it is the fast behavioral gate (about 1.5 minutes). golden.yml is now also candidate-capable for explicit/manual investigation, but remains a separate reference suite. It takes about 6.5 minutes and compares strict numerical output against the pinned dependency set; a candidate Fortio can produce small floating-point differences without a behavioral regression. A manual candidate golden run showed exactly that kind of small reference drift, so making it the blocking candidate gate would conflate those two purposes. benchmark.yml remains an independent performance comparison.

So: yes, runtime is part of the reason, but the stronger reason is that the golden and compatibility gates answer different questions.

@krystophny

Copy link
Copy Markdown
Member Author

Dependency follow-up: this branch now pins libneo bridge 9f1e9dcf5ab6800213415eac485e85a6d395f72a from libneo PR itpplasma/libneo#413, which consumes standalone Fortio 11afd0bd1af0c99ea4e9a1c0df683dbdcc299b69 from Fortio PR lazy-fortran/fortio#68. No system HDF5 link is introduced.

@krystophny

Copy link
Copy Markdown
Member Author

Correction: the compatible main-line libneo bridge is PR itpplasma/libneo#414 at commit 7262c32bf0406594a8ef27e84654d249aef2cb86. This replaces the FortFEM-specific bridge reference; it consumes standalone Fortio PR lazy-fortran/fortio#68. The consumer branch has been updated and CI restarted.

@GeorgGrassler

GeorgGrassler commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

@krystophny all fine from my side, just two things that confused me:

  • in you last response,

(c) The automatic direct compatibility target is test.yml: it is the fast behavioral gate (about 1.5 minutes). golden.yml is now also candidate-capable for explicit/manual investigation, but remains a separate reference suite. It takes about 6.5 minutes and compares strict numerical output against the pinned dependency set; a candidate Fortio can produce small floating-point differences without a behavioral regression. A manual candidate golden run showed exactly that kind of small reference drift, so making it the blocking candidate gate would conflate those two purposes. benchmark.yml remains an independent performance comparison.

So: yes, runtime is part of the reason, but the stronger reason is that the golden and compatibility gates answer different questions.

you said that you did not want golden.ymlas a candidate gate, but now you added the manual dispatch functionality anyway (8dc510a)? Again, not a problem, just confused cause you said A, but seemed ot have done B.

  • is there a specific reason why @phizenz is also part of this review?

I wanna make it clear that I do not require any changes, I just want those questions answered and they I clear the review and merge.

@krystophny

Copy link
Copy Markdown
Member Author

@GeorgGrassler Thanks — the distinction is automatic versus manual. test.yml is the registered automatic compatibility gate. golden.yml accepts a candidate ref only through explicit manual dispatch for investigation; it is not an automatic or blocking promotion gate.

Philipp was requested only as an optional second pair of eyes and is not required for approval or merge. I have removed that extra review request to make this unambiguous.

@krystophny
krystophny removed the request for review from phizenz August 31, 2026 12:54
@GeorgGrassler
GeorgGrassler disabled auto-merge August 31, 2026 15:01
@GeorgGrassler
GeorgGrassler merged commit 3282d65 into main Aug 31, 2026
5 checks passed
@GeorgGrassler
GeorgGrassler deleted the agent/fortio-direct-gate branch August 31, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants