From 12891ce61695fa8f4ed9b5ecb738130a0d107c40 Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 02:06:23 +0200 Subject: [PATCH 1/8] Add first round of tests, but the don't actually catch the bug yet --- tests/test_rpn.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/test_rpn.py diff --git a/tests/test_rpn.py b/tests/test_rpn.py new file mode 100644 index 00000000..677a04c8 --- /dev/null +++ b/tests/test_rpn.py @@ -0,0 +1,22 @@ +from cheetah.converters.utils import rpn + + +def test_valid_rpn_expression(): + """ + Test that a valid RPN expression without nesting is correctly recognised as a valid + RPN expression. + """ + expression = "'2 3 +'" + + assert rpn.is_valid_expression(expression) + + +def test_falsely_validated_normal_expression(): + """ + Tests that the expression `"ldsp2h +dldsp17h +lblxsph/2-lbxsph/2"`, which was + falsely recognised as a valid RPN expression in a previous version of Cheetah, is + correctly recognised as invalid. + """ + expression = "'ldsp2h +dldsp17h +lblxsph/2-lbxsph/2'" + + assert not rpn.is_valid_expression(expression) From b8170e78b5727035c8bf1aa453cacdf9eb4d8c91 Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 02:16:23 +0200 Subject: [PATCH 2/8] Fix up tests --- tests/test_rpn.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/test_rpn.py b/tests/test_rpn.py index 677a04c8..e5718eb5 100644 --- a/tests/test_rpn.py +++ b/tests/test_rpn.py @@ -6,17 +6,28 @@ def test_valid_rpn_expression(): Test that a valid RPN expression without nesting is correctly recognised as a valid RPN expression. """ - expression = "'2 3 +'" + expression = "2 3 +" assert rpn.is_valid_expression(expression) +def test_valid_rpn_expression_with_single_quotes(): + """ + Test that a valid RPN expression that has single quotes around it and should + therefore not be recognised as valid because these should have been stripped off + before calling the function. + """ + expression = "'2 3 +'" + + assert not rpn.is_valid_expression(expression) + + def test_falsely_validated_normal_expression(): """ Tests that the expression `"ldsp2h +dldsp17h +lblxsph/2-lbxsph/2"`, which was falsely recognised as a valid RPN expression in a previous version of Cheetah, is correctly recognised as invalid. """ - expression = "'ldsp2h +dldsp17h +lblxsph/2-lbxsph/2'" + expression = "ldsp2h +dldsp17h +lblxsph/2-lbxsph/2" assert not rpn.is_valid_expression(expression) From 90e5de13625d4fa16e3d90b6b9c8b5eed08d7f5a Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 09:11:27 +0200 Subject: [PATCH 3/8] Fix RPN functions in isolation --- cheetah/converters/utils/rpn.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cheetah/converters/utils/rpn.py b/cheetah/converters/utils/rpn.py index 2ccdd2e4..f4818cd0 100644 --- a/cheetah/converters/utils/rpn.py +++ b/cheetah/converters/utils/rpn.py @@ -3,7 +3,7 @@ def is_valid_expression(expression: str) -> bool: """Checks if expression is a reverse Polish notation.""" - stripped = expression[1:-1].strip() + stripped = expression.strip() return stripped[-1] in "+-/*" and len(stripped.split(" ")) == 3 @@ -13,5 +13,5 @@ def eval_expression(expression: str, context: dict) -> Any: NOTE: Does not support nested expressions. """ - splits = expression[1:-1].strip().split(" ") + splits = expression.strip().split(" ") return eval(" ".join([splits[0], splits[2], splits[1]]), context) From 08ae078d6632b3c0b482fe57613ed5f2417803e2 Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 09:21:54 +0200 Subject: [PATCH 4/8] Strip quotes in namelist loader --- cheetah/converters/utils/fortran_namelist.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cheetah/converters/utils/fortran_namelist.py b/cheetah/converters/utils/fortran_namelist.py index 09064240..f052bf33 100644 --- a/cheetah/converters/utils/fortran_namelist.py +++ b/cheetah/converters/utils/fortran_namelist.py @@ -136,6 +136,8 @@ def evaluate_expression(expression: str, context: dict) -> Any: # lattice file. I'm not sure this replacement will lead to the intended # behaviour. expression = re.sub(r"abs\(", r"abs_func(", expression) + # Remove surrounding quotes + expression = re.sub(r"\"([^\"]+)\"", r"\1", expression) return ( eval(expression, context) From ee75000802aa9cfd9df2a0e3e664c65a69d50d37 Mon Sep 17 00:00:00 2001 From: Christian Hespe Date: Mon, 21 Oct 2024 10:04:55 +0200 Subject: [PATCH 5/8] Add missing arguments to Elegant cavity test file --- tests/resources/cavity.lte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/resources/cavity.lte b/tests/resources/cavity.lte index 0fc46540..d21ed886 100644 --- a/tests/resources/cavity.lte +++ b/tests/resources/cavity.lte @@ -1,4 +1,4 @@ C1E: EMATRIX, L = 0, ORDER = 1, C2=-0.0027, C4=-0.150, R11=1, R21=0.04, R22=1, R23=0.003, R33=1, R41=0.003, R43=-0.04, R44=1, R55=1, R66=1 -C1: RFCA, L = 0.7, PHASE = 90.000000, VOLT = 16175000.000000, FREQ = 1200000000.000000 +C1: RFCA, L = 0.7, PHASE = 90.000000, VOLT = 16175000.000000, FREQ = 1200000000.000000, CHANGE_P0 = 1, END1_FOCUS = 1, END2_FOCUS = 1, BODY_FOCUS_MODEL = "SRS" cavity: line=(C1E,C1) From 00d841e64d80094e38618e0989d72658d027a17f Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 10:10:10 +0200 Subject: [PATCH 6/8] Fix Fortran namelist parsing error when trying to evaluate a string --- cheetah/converters/utils/fortran_namelist.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cheetah/converters/utils/fortran_namelist.py b/cheetah/converters/utils/fortran_namelist.py index f052bf33..d690b4e3 100644 --- a/cheetah/converters/utils/fortran_namelist.py +++ b/cheetah/converters/utils/fortran_namelist.py @@ -144,6 +144,9 @@ def evaluate_expression(expression: str, context: dict) -> Any: if not rpn.is_valid_expression(expression) else rpn.eval_expression(expression, context) ) + except NameError: + # The evaluation could not be evaluated, so it is probably a string + return expression except SyntaxError: if not ( len(expression.split(":")) == 3 or len(expression.split(":")) == 4 From 242665b370c052cec13e0432a2e3372ef105d462 Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 11:20:16 +0200 Subject: [PATCH 7/8] Remove non-working fix for Bmad LCLS import issue --- cheetah/converters/utils/fortran_namelist.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cheetah/converters/utils/fortran_namelist.py b/cheetah/converters/utils/fortran_namelist.py index d690b4e3..09064240 100644 --- a/cheetah/converters/utils/fortran_namelist.py +++ b/cheetah/converters/utils/fortran_namelist.py @@ -136,17 +136,12 @@ def evaluate_expression(expression: str, context: dict) -> Any: # lattice file. I'm not sure this replacement will lead to the intended # behaviour. expression = re.sub(r"abs\(", r"abs_func(", expression) - # Remove surrounding quotes - expression = re.sub(r"\"([^\"]+)\"", r"\1", expression) return ( eval(expression, context) if not rpn.is_valid_expression(expression) else rpn.eval_expression(expression, context) ) - except NameError: - # The evaluation could not be evaluated, so it is probably a string - return expression except SyntaxError: if not ( len(expression.split(":")) == 3 or len(expression.split(":")) == 4 From 995dcacf9d5f4f7ee2da8f352541208e4c97cf1f Mon Sep 17 00:00:00 2001 From: Jan Kaiser Date: Mon, 21 Oct 2024 12:07:44 +0200 Subject: [PATCH 8/8] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae4078ac..65c9c24d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ This is a major release with significant upgrades under the hood of Cheetah. Des - Port Bmad-X tracking methods to Cheetah for `Quadrupole`, `Drift`, and `Dipole` (see #153, #240) (@jp-ga, @jank324) - Add `TransverseDeflectingCavity` element (following the Bmad-X implementation) (see #240, #278) (@jp-ga, @cr-xu, @jank324) - `Dipole` and `RBend` now take a focusing moment `k1` (see #235, #247) (@hespe) -- Implement a converter for lattice files imported from Elegant (see #222, #251, #273) (@hespe) +- Implement a converter for lattice files imported from Elegant (see #222, #251, #273, #281) (@hespe, @jank324) ### 🐛 Bug fixes