Bmad conversion bugfixes -- take 2 for #663 - #683
Open
jank324 wants to merge 16 commits into
Open
Conversation
This reverts commit f4796e8.
jank324
marked this pull request as draft
August 14, 2026 09:33
- Move alias and type metadata extraction to bmad.py to preserve generic namelist parser separation - Parse quoted string literals in evaluate_expression to prevent spurious PhysicsWarnings - Simplify overlay and group skipping with a unified control regex and remove dead code - Fix resolution of typed non-wildcard property assignments (type::name[prop]) - Use standard Bmad attribute phi0 for crab cavity phase - Add and refine docstrings and test assertions across test suite
…esy-ml/cheetah into redo-663-bmad-conversion-fixes
jank324
commented
Aug 18, 2026
Comment on lines
-313
to
-354
| def define_overlay(line: str, context: dict) -> dict: | ||
| """ | ||
| Define an overlay in the context. | ||
|
|
||
| :param line: Line of an overlay definition to be parsed. | ||
| :param context: Dictionary of variables to define the overlay in and from which to | ||
| read variables. | ||
| :return: Updated context. | ||
| """ | ||
|
|
||
| expression_match = re.fullmatch(OVERLAY_EXPRESSION_BASED_PATTERN, line) | ||
| knot_match = re.fullmatch(OVERLAY_KNOT_BASED_PATTERN, line) | ||
|
|
||
| if knot_match: | ||
| overlay_name = knot_match.group(1).strip() | ||
| overlay_definition = knot_match.group(2).strip() | ||
| overlay_variable = knot_match.group(3).strip() | ||
| overlay_x_knot = knot_match.group(4).strip() | ||
|
|
||
| context[overlay_name] = { | ||
| "overlay_definition": overlay_definition, | ||
| "overlay_variable": overlay_variable, | ||
| "overlay_x_knot": overlay_x_knot, | ||
| } | ||
| elif expression_match: | ||
| overlay_name = expression_match.group(1).strip() | ||
| overlay_definition = expression_match.group(2).strip() | ||
| overlay_variables = expression_match.group(3).strip() | ||
| if expression_match.group(4) is not None: | ||
| overlay_parameters = expression_match.group(4).strip()[1:].strip() | ||
| else: | ||
| overlay_parameters = None | ||
|
|
||
| context[overlay_name] = { | ||
| "overlay_definition": overlay_definition, | ||
| "overlay_variables": overlay_variables, | ||
| "overlay_parameters": overlay_parameters, | ||
| } | ||
| else: | ||
| raise ValueError(f"Overlay definition {line} not understood.") | ||
|
|
||
| return context |
Member
Author
There was a problem hiding this comment.
I'm not sure how good of an idea it is to remove this now, but it seems like currently this is dead code ... and who knows if it actually works. I don't remember.
jank324
commented
Aug 18, 2026
Co-authored-by: Jan Kaiser <jan.kaiser.email@googlemail.com>
…esy-ml/cheetah into redo-663-bmad-conversion-fixes
jank324
commented
Aug 18, 2026
Co-authored-by: Jan Kaiser <jan.kaiser.email@googlemail.com>
…esy-ml/cheetah into redo-663-bmad-conversion-fixes
Contributor
There was a problem hiding this comment.
Pull request overview
This PR redoes the Bmad conversion bugfix work from #663, focusing on more robust parsing of Bmad/Fortran-style expressions and property assignments, and on preserving alias/type metadata on converted Cheetah elements.
Changes:
- Extend infix expression parsing to support unary
+/-and scientific-notation tokens (e.g.0.750e-3). - Update the Fortran namelist/Bmad line parser to (a) skip overlay/group control definitions and (b) better resolve wildcard + typed property assignments (e.g.
lcavity::l0a[...],l0*[...]), plus support quoted string literals. - Improve Bmad element conversion by propagating
alias/typeinto elementmetadata, handling missinge1on bends, and addingcrab_cavityconversion; add/extend test fixtures and changelog entry.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cheetah/converters/utils/infix.py |
Adds unary sign operators and scientific-notation tokenization improvements. |
cheetah/converters/utils/fortran_namelist.py |
Skips overlay/group definitions, supports quoted strings, and improves wildcard/typed property assignment resolution. |
cheetah/converters/bmad.py |
Propagates alias/type into metadata, makes sbend e1 optional, and adds crab_cavity conversion. |
tests/test_infix.py |
Adds tests for unary-minus-before-function-call and scientific notation parsing. |
tests/test_fortran_namelist.py |
New tests covering context eval, quoted strings, typed assignments, and skipping control definitions. |
tests/test_bmad_conversion.py |
Adds an end-to-end conversion test for the reduced CU_HXR fixture. |
tests/resources/lcls/cu_hxr.lat.bmad |
New reduced CU_HXR-like Bmad lattice fixture used by conversion tests. |
tests/resources/lcls/cu_hxr_overlays_and_fixers.bmad |
New include file containing representative overlay/group/superimpose constructs. |
CHANGELOG.md |
Documents the Bmad conversion bugfixes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jank324
marked this pull request as ready for review
August 18, 2026 14:23
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.
Description
Redo #663 to fix issue with that PR.
Motivation and Context
Types of changes
Checklist
flake8(required).pytesttests pass (required).pyteston a machine with a CUDA GPU and made sure all tests pass (required).Note: We are using a maximum length of 88 characters per line.