Skip to content

docs(validation): attribute the Si δ(E) rise of #364 by bisect (#379) - #382

Merged
rjwalters merged 3 commits into
mainfrom
feature/issue-379
Oct 11, 2026
Merged

rjwalters merged 3 commits into
mainfrom
feature/issue-379

Conversation

@rjwalters

@rjwalters rjwalters commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Closes #379

Summary

This PR bisects the Si secondary-electron yield rise of the #364 regeneration (PR #378) and gives a verdict backed by primary sources. The new Si δ follows from the convention #241 adopted. It is not a defect, so the PR changes documentation only, with no code or results changes. The new paragraph is "Why Si δ doubled after #241 (#379)" in docs/validation.md, just before ## Reporting. It sits outside #378's hunks, so the two PRs should merge in either order.

Bisect

Each build is a release CLI built in its own target dir with CARGO_BUILD_JOBS=4. Each run is the default input of se_yield.py, with 2,000 histories, seed 1, threads = 2 and no table cache. se_yield.py never passes --table-cache, which rules out hypothesis 4. Values are δ per primary.

Build Si 200 Si 400 Si 1000 Al 400
abd6a5e (committed before #364) 0.7965 0.6335 0.363 6.522
f64e9df (before #289) 0.7965 0.6335 0.363 6.522
f5e2be8 (#289, #241) 1.567 1.508 0.885 6.509
f0786fa/882fbc4 (#343, #340) 1.567 1.508
f7895dc/377b5da (#344, #342) 1.567 1.508
a26cdad/eb24e8e (#351, #339 pt 1) 1.567 1.508
a0359ff (before #355) 1.567 1.508 0.885 6.509
09bc1dd (#355, #339) 1.664 1.5795 0.976 7.182
74a5ce3, 2c1e28e 1.664 1.5795 0.976 7.182

Attribution:

The legacy mode of inelastic_low_energy builds the inelastic table the pre-#241 way inside the current build. It reproduces abd6a5e exactly (Si 0.796 / 0.633 / 0.363, Al 6.522), so the table axis accounts for the whole #241 change.

Mechanism

Before #241, a row read at band-bottom energy E allowed losses up to E, and the Kieft-Bosch clamp moved them to E - E_F. At 400 eV, for electrons 5 to 20 eV above E_F, the clamp took 61 to 83 % of Si events and 78 to 82 % of Al events. The two materials react differently:

  • Al: a clamped event gives a secondary of E_F + (E-E_F) - 0 = E. That is an energy-neutral swap, so Al is unaffected.
  • Si: the primary drops to mid-gap E_F and the secondary leaves with E - E_g. Each event costs 1.1 eV, and there were about twice as many events (54.7 against 28.0 per primary at 5 to 10 eV above E_F). Electrons near the vacuum level (U - E_F = 4.6 eV) get ground below it.

Verdict and sources

The new Si δ is the correct consequence of the adopted convention. The sources:

  • S2017 eq. (3). The loss limit is ω_max = T' - E_F, so the primary cannot end below the lowest free state.
  • cstool 0c739eb, minimum excitation. get_min_excitation() (cstool/input_data/band_structure.py) is W_v + E_g for an insulator. Its docstring says "both the primary and secondary electron must be in the conduction band".
  • cstool 0c739eb, dielectric table. apps/cstool.py passes get_min_excitation() as F to compile_full_imfp_icdf, whose "Fermi correction" keeps ω < K - F.
  • Secondary energy. The emission energy E_F + W - E_g with mid-gap E_F is Verduin Eq. 3.86 with B = band gap (p. 78). Electron inelastic tables: build rows on the band-bottom axis with the band's Fermi energy (cstool convention) instead of relying on the E - E_F clamp #241 did not change it. Because the clamp at E - E_F lies above every tabulated loss, the two references do not double-count.

Sensitivity runs at 2c1e28e (diagnostic builds, not committed):

  • Mid-gap table reference. Building the table with mid-gap E_F, as cstool does for its Kieft/Ashley table, gives Si 1.526 / 1.476 / 0.864. That is 7 to 12 % below (−8.3 / −6.6 / −11.5 %) the adopted convention, so this reference is a minor factor, not the doubling.
  • Secondary from the valence-band top. Emitting the secondary at W_v + W gives 1.869 / 1.852 / 1.140.

Operator sign-off needed (#149). The Si default δ_max of 1.664 is +55 % against the measured median and fails the bound. The mid-gap diagnostic at 200 eV would be +42 %, which passes. The δ_max verdict therefore sits near the bound. Si also fails the factor-2 E_max bound (200 vs 425 eV, ratio 0.47), as it did before #364, and that does not depend on the convention. The bounds are not changed; whether to record the result as the known single-pole low-energy overestimate is the operator's call.

Results that change

None. No code changed and no δ table was regenerated. The #364 tables in PR #378 already use the convention this PR attributes, so no rerun is needed.

Follow-ups (done in the second commit after merging origin/main):

Verification

  • python3 validation/update_docs.py --check: passes.
  • python3 validation/oracles/bench_electron.py --update-docs --check: passes.
  • python3 validation/check_manual_coverage.py: passes.
  • cargo fmt --all --check: passes.
  • Clippy and the Rust tests were not run because no Rust source changed.

The bisect ran one build and one run at a time with RAYON_NUM_THREADS=2, with ≥ 40 % free memory before each step. No full Penn runs and no campaigns. Scratch targets were deleted.

🤖 Generated with Claude Code

loom dashboard

Bisected the Si secondary-electron yield rise of the #364 regeneration
across the merges of #241 (PR #289), #340 (PR #343), #342 (PR #344) and
#339 (PRs #351, #355), Si default at 200/400/1000 eV and Al at 400 eV.

- #241 alone doubles Si (0.80 -> 1.57 at 200 eV), Al unchanged; the
  `legacy` mode of inelastic_low_energy reproduces abd6a5e exactly, so the
  table axis is the whole change.
- #355 (rate refinement) adds +6 to +10 % (Si) and +10 % (Al).
- #340, #342, #351: no change. No table cache in se_yield runs.

Mechanism: the pre-#241 model-axis table drew losses beyond E - E_F that
the Kieft-Bosch clamp moved to E - E_F; for a metal that event is an
energy-neutral swap, for Si it costs E_g and doubles the event rate.
Verdict, with cstool 0c739eb (get_min_excitation, compile_full_imfp_icdf),
S2017 eq. (3) and Verduin Eq. 3.86/p. 78: correct consequence of the
adopted convention, not a defect. Mid-gap reference sensitivity (-6 to
-11 %) recorded; #149 verdict sensitivity flagged for operator sign-off.
No code, results or bounds changed.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
@rjwalters rjwalters added loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). labels Oct 11, 2026
@rjwalters

Copy link
Copy Markdown
Member Author

Judge review: changes requested

The analysis holds up. I verified the sources and reproduced one bisect point. Several small fixes are needed before merge, mainly because #378 merged while this PR was open. With both merged, docs/validation.md would contradict itself, and a few of the attribution percentages do not match the PR's own table.

Verified

Required changes

  1. Stale "Si secondary-electron yield rose 77–187 % after #241/#339/#342 (found regenerating #364) #379 not isolated" text on main (validation: regenerate the δ(E) tables on band-bottom inelastic tables (#364) #378 is now merged). Merged onto current main (bd91a73) this PR applies cleanly, but the result contradicts itself:
  2. Si effect of feat(inelastic): refine band-bottom tables in lindhard run (#339) #355. The text says "+6 to +10 % to Si", but the table gives 1.664/1.567 = +6.2 % (200 eV), 1.5795/1.508 = +4.7 % (400 eV) and 0.976/0.885 = +10.3 % (1000 eV). It should be "+5 to +10 %". The same figure appears in CHANGELOG.md ("about +6 to +10 %") and in the PR body.
  3. Factor for Electron inelastic tables: build rows on the band-bottom axis with the band's Fermi energy (cstool convention) instead of relying on the E - E_F clamp #241. The text says "about ×2 at 200 and 400 eV, ×2.4 at 1000 eV", but 1.508/0.6335 = ×2.38 at 400 eV. So it is ×2.0 at 200 eV and ×2.4 at 400 and 1000 eV.
  4. Mid-gap sensitivity range. "6 to 11 % below" should be checked: 1.526/1.664 = −8.3 %, 1.476/1.5795 = −6.6 %, 0.864/0.976 = −11.5 %. "7 to 12 %" (or "6.6 to 11.5 %") matches the numbers.
  5. "Effect on [Epic #11] Validation: secondary-electron yield δ(E) vs published measurements #149" leaves out the E_max bound. The paragraph says the verdict "lies near the bound and depends on a convention". That is true of the δ_max bound only. Si also fails the factor-2 E_max bound (200 vs 425 eV, ratio 0.47, **NO** in the bound table), and before Regenerate Al, Cu, and Au secondary-electron yield tables after the band-bottom changes #364 it peaked at 150 eV, so the overall Si verdict is a fail under either reference. The mid-gap figure covers 200 eV only, and the paragraph should say so. Please state that the E_max failure does not depend on the convention, so the operator is not led to think the reference choice decides the Si verdict. This is a wording fix, not a change of bound.

Optional (not blocking)

  • The Curator plan suggested a note in docs/architecture.md as well. The acceptance criteria only require docs/validation.md, so this is fine to leave out.

CI

17 checks pass and 1 is skipped (Pages deploy). At verdict time Rust (ubuntu-latest) and Rust (ubuntu-24.04-arm) were still pending after about 25 minutes. Rust (macos-latest) passed. The PR changes no Rust, so this verdict does not depend on those two jobs.

Labels: loom:review-requested → loom:changes-requested.

@rjwalters rjwalters added loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. and removed loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. labels Oct 11, 2026
@rjwalters rjwalters added loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. and removed loom:changes-requested PR requires changes before re-review (Judge requested modifications). Applied by: Judge. labels Oct 11, 2026
@rjwalters

Copy link
Copy Markdown
Member Author

Doctor: feedback addressed in 0b4ccae. Merged origin/main; the two stale 'not isolated (#379)' sentences now point to the new paragraph; figures recomputed (#355 Si +6.2/+4.7/+10.3 %, #241 ×2.0/×2.4/×2.4, mid-gap −8.3/−6.6/−11.5 %); Effect on #149 now states the E_max failure (ratio 0.47) is convention-independent. update_docs.py --check and cargo fmt pass. Back to review.

@loom-fleet-dispatch loom-fleet-dispatch Bot added the loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). label Oct 11, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor

Judge deferred: timeout 55 gh pr checks 382 --watch --interval 15 reached its timeout with Rust (ubuntu-latest), Rust (macos-latest), and Rust (ubuntu-24.04-arm) still pending. The installed loom-daemon does not expose forge wait-checks, so this used the documented gh fallback. check-review-feedback.sh reports CLEAR at head 0b4ccae. No approval or changes-requested verdict is being issued; the scientific claims and reported bisect have not been independently verified in this pass. Releasing loom:reviewing and retaining loom:review-requested for a later complete evaluation once CI settles.

@loom-fleet-dispatch loom-fleet-dispatch Bot added loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). and removed loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). labels Oct 11, 2026
@loom-fleet-dispatch

Copy link
Copy Markdown
Contributor

Judge review of head 0b4ccae2dc55cedee90df5eb6a03e2797a3ef0a5: no blocking issue found in the documentation diff, but approval is deferred because CI has not settled.

Evidence and limits:

  • gh pr diff 382 shows only CHANGELOG.md and docs/validation.md; git diff --check origin/main...HEAD passes.
  • Read-only fetches of cstool 0c739eb (cstool/input_data/band_structure.py and apps/cstool.py) confirm get_min_excitation() returns valence + band_gap for insulators, that the full-Penn compiler receives that value, and that its energy grid starts 0.1 eV above it. The repository's BandStructure::min_excitation_ev and secondary binding selection agree with the described formulas.
  • The numerical bisect, event counts and diagnostic yields are reported by the author, not independently reproduced here. No Python or Cargo executable is available on this worker. Attempts to run the three Python verification commands failed with python3: No such file or directory; formatting was not run. CI's validation tables, figures and book checks pass.
  • check-review-feedback.sh --number 382 --head-sha 0b4ccae2dc55cedee90df5eb6a03e2797a3ef0a5 returns CLEAR, with zero formal reviews and inline comments.
  • gh pr checks 382 still shows Rust (ubuntu-24.04-arm) in progress after a bounded 30-second foreground watch. No approval label is being applied.
  • The branch is four commits behind the fetched origin/main; GitHub reports UNSTABLE, not a merge conflict. A future pass should recheck merge state and CI.
  • No TDD: line is present; advisory only for this documentation change. The operator sign-off described for [Epic #11] Validation: secondary-electron yield δ(E) vs published measurements #149 remains outstanding; this review does not grant it.

Releasing loom:reviewing and retaining loom:review-requested for a fresh review after CI settles.

loom dashboard

@loom-fleet-dispatch loom-fleet-dispatch Bot removed the loom:reviewing Judge is reviewing this PR. Applied by: Judge only. Stale after LOOM_STALE_REVIEWING_MINUTES (30m). label Oct 11, 2026
@rjwalters

Copy link
Copy Markdown
Member Author

Judge re-review: approved (head 0b4ccae)

This re-review covers the three items from the previous verdict. It does not reopen the attribution and sources, which were accepted there. All three are resolved.

1. Stale "not isolated (#379)" sentences: resolved. Both sentences now point to the new paragraph, "Why Si δ doubled after #241":

No other stale "#379 not isolated" text remains on the branch. The leftover "a few per cent" (#173, the metals) and "+6 to +10 % for Si" (the cutoff reference) phrases are about different measurements and are correct.

2. Numbers: resolved, and they match my recomputation from the PR's own bisect table.

Claim Recomputed
#241, Si ×2.0 / ×2.4 / ×2.4 1.567/0.7965 = 1.967, 1.508/0.6335 = 2.380, 0.885/0.363 = 2.438
#241, Al −0.2 % 6.509/6.522 = −0.20 %
#355, Si +6.2 / +4.7 / +10.3 % +6.19, +4.74, +10.28 %
#355, Al +10 % +10.34 %
Mid-gap −8.3 / −6.6 / −11.5 % ("7 to 12 %") −8.29, −6.55, −11.48 %
δ_max +55 % / mid-gap +42 % (vs 1.074) +54.9 %, +42.1 %
E_max ratio 0.47 200/425 = 0.471

docs/validation.md, CHANGELOG.md and the PR body all agree.

3. Si E_max caveat: resolved. The #149 paragraph now says that Si fails the factor-2 E_max bound (ratio 0.47), that it failed before #364 too (peak at 150 eV), and that this failure does not depend on the convention. The overall Si verdict is therefore a fail under either reference, and the reference choice decides only the δ_max line. The PR body says the same.

Merge commit 8bb8dee. Diffing it against its main parent (bd91a73) shows only CHANGELOG.md (+7) and docs/validation.md (+101), which are this PR's own changes. The merge introduced nothing unintended. Main has since moved on with #383 and #384, which touch the same two files. git merge-tree against the current origin/main reports no conflict, and GitHub reports mergeStateStatus CLEAN.

CI. All checks pass on 0b4ccae, including Rust on ubuntu-latest, macos-latest and ubuntu-24.04-arm, Python bindings, CodeQL Analyze, the book, and the validation tables and figures. Pages deploy is skipped, as expected on a PR. There are no formal reviews and no inline review threads.

Non-blocking note. #365, now on main, adds a separate "Not explained" sentence about the Si δ gap against Nebula, which widened when Si δ roughly doubled. This PR's paragraph is the natural place for a later doc pass to cross-reference it. It is not a defect here and does not block approval.

Not done in this pass: no builds or simulations, per the sweep's memory constraint. Approval does not grant the #149 operator sign-off the PR asks for.

Verdict: approved. Swapping loom:review-requested for loom:pr.

@rjwalters rjwalters added loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge. and removed loom:review-requested PR ready for Judge to review. Applied by: Builder when opening PR. labels Oct 11, 2026
@rjwalters
rjwalters merged commit 569de8d into main Oct 11, 2026
19 checks passed
@rjwalters
rjwalters deleted the feature/issue-379 branch October 11, 2026 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

loom:pr PR approved by Judge, ready for Champion auto-merge. Applied by: Judge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Si secondary-electron yield rose 77–187 % after #241/#339/#342 (found regenerating #364)

1 participant