Model the set and sets Commands - #84
Open
ax3l wants to merge 1 commit into
Open
Conversation
A `facility` entry keyed by a reserved node keyword is a command, not a
lattice element. `sets` carries a sequence, so it never got past the
`{name: properties}` unpacker; `set` did get past it and was read as
whichever element of the union happened to accept it (a BeamBeam).
Add a `commands` package holding SetCommand and SetsCommand, per the
standard's Setting Parameters section, and recognize their keywords in
the facility list only. Values are recorded verbatim, whether a number
or an expression string: this is the exact representation, so nothing is
evaluated.
Clears unit_tests/sets/sets_compact.pals.yaml from the known-failures
list, and makes set_pattern.pals.yaml and set_single_definition.pals.yaml
pass for the right reason. Since a bare PASS turned out to prove so
little, the corpus validator grows a spot-check table.
Co-Authored-By: Claude Opus 5 (1M context) <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.
A
facilityentry keyed by a reserved node keyword is a command, not a lattice element. This models the two the standard specifies in its Setting Parameters section.The problem
setscarries a sequence, so it never got past the{name: properties}unpacker:setis worse, because it did not error. It satisfies the{name: properties}shape, and sinceBaseElementdoes not forbid extras and the facility union is not discriminated, it landed on whichever member accepted it:So two files in the standard's corpus were reporting
PASSwhile being read wrongly.The change
src/pals/commands/package, sibling tokinds/andparameters/, holdingSetCommand,SetsCommandand a sharedFacilityCommandbase.all_commands.pyis the one module that maps a keyword to its model, mirroringkinds/all_elements.py;unpack_element_itemsgainsallow_commands, which only the facility passes.BeamLine.line,Lattice.branchesandUnionEle.elementskeep rejecting commands, as the standard placessetunderfacility;0.25) or an expression (2*PARAMETER + atan(SELF.BendP.g_ref)). This is the exact representation, so nothing is evaluated;absolute_error/relative_errordefault toNonerather than the documented0, so a property the file did not write stays out of the output and round trips stay faithful.The compact form desugars into
SetCommandpairs, so a reader can treat both forms alike, and is written back out in its compact form.Result
unit_tests/sets/sets_compact.pals.yamlleaves the known-failures list, andset_pattern.pals.yamlandset_single_definition.pals.yamlnow pass for the right reason.Since a bare
PASSturned out to prove so little,validate_standard_examples.pygrows a spot-check table alongside the existingcheck_fodo, and the new entry asserts thesetsnode really is aSetsCommandwith its two pairs.tests/test_commands.pycovers both forms, the error terms, YAML and JSON round trips, documents assembled from Python objects, and the structural errors.Verified
pytest tests -v— 63 passed, 2 skipped (was 49 passed, 2 skipped)pre-commit run --all-files,python examples/fodo.pyDeliberately not in this PR
constantsandvariablesstill misparse the same waysetdid — the map form silently becomes an element, the sequence form raises.variablesis also, as far as I can tell, not documented in the standard'ssource/*.md; it appears only in the examples corpus and pals-cpp.inline_expressions.pals.yamlstays a known failure. Thevariablessequence form alone would not clear it: it also needs expressions accepted where a parameter is typed as a number (length: 0.1*log(abs(b_var)),Kn1: expr(...)) and akind: constantelement.expand_latticeandControllerare unmodeled.PlaceholderNamerather than erroring.Q1insets_compact.pals.yamlis aQuadrupolewith no multipole group, so it tripsvalidate_at_least_one_multipoleand is read as a bare reference, losingkindandlength. Worth its own issue — the fix is likely to make the element union discriminated onkind.🤖 Generated with Claude Code