Default a branch's inherit to its own name; report empty branches#76
Merged
Conversation
A branch's `inherit` is optional and defaults to the branch's own name
(lattice-construction.md, s:lattice.construct), so
branches:
- ln:
periodic: false
is the branch `ln` built from the BeamLine `ln`. Expansion reached a
definition only through a bare `- ln` or an explicit `inherit`, so this
form expanded to nothing at all -- silently. The missing elements then
surfaced somewhere unrelated and far downstream, as a controller target
that "matches nothing in the expanded lattice".
Supply the default in `default_branch_inherit`, before expand() runs, by
writing the entry's key back out as an `inherit`. Appending it leaves the
branch's own keys ahead of the inherited ones, which is what keeps
`periodic` an override.
`check_branches_expanded` then reports any branch that comes out of
expansion holding no elements, so this class of mistake is diagnosed
where it happens rather than wherever a later check first trips over it.
Also: `where()` in pals_check joins an element and its parameter group
with the `>` the parameter paths use -- `element 'beginning_b1>ReferenceP'`
rather than `element 'beginning_b1' ReferenceP`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A branch's
inheritis optional and "Default is the name of the Branch" (lattice-construction.md, s:lattice.construct). Sois the branch
lnbuilt from the BeamLineln, overriding itsperiodic. Expansion reached a definition only through a bare- lnor an explicitinherit:, and never consulted the entry's own key — so this form expanded to nothing at all, silently.The missing elements then surfaced somewhere unrelated. On a real file the only sign was:
which is true but says nothing useful, and it also masked a genuine error in the file (a line referring to an element that did not exist).
The fix
default_branch_inheritruns beforeexpand()and writes a keyed branch entry's own key back out as aninherit, when the branch has neither an explicitinheritnor alineof its own and the name is defined. Appending it leaves the branch's own keys ahead of the inherited ones, which is what keepsperiodican override. Fork-built branches copy theirto_lineoutright and never pass through here.check_branches_expandedruns after expansion — so root and forked branches alike have had their chance to fill in — and reports any branch holding no elements:branch 'ln': no root BeamLine 'ln' is definedwhen the name was defaulted from the key and resolves to nothing;branch 'ln': expanded to no elementsotherwise. An explicitinheritnaming nothing is left to expand()'s existinginherit: 'x' is not defined.Without this, an empty branch is diagnosed only wherever some later check first trips over it.
Also
where()inpals_check.cppnow joins an element with its parameter group using the same>the parameter paths use:rather than
element 'beginning_b1' ReferenceP: .... A group with no owning element still renders bare.Tests
Four new cases (suite: 221 cases / 1162 assertions, all passing):
test_expansion.cpp— a branch with noinheritpicks up its named root line and keeps itsperiodicoverride; an explicitinheritstill wins over the branch key; an empty branch is reported, both the undefined-root and empty-line variants.test_controllers.cpp— the reported shape end to end:q>MagneticMultipoleP.Kn1resolves and drives the parameter.Verified as real regressions by stubbing out both new calls: 3 of the 4 fail without the fix. The explicit-
inheritcase is a guard against the default overreaching, so it passes either way.🤖 Generated with Claude Code