From af2fb2efb9df53dc5ee0839a17474e1e84f47bc8 Mon Sep 17 00:00:00 2001 From: Christopher Mayes <31023527+ChristopherMayes@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:03:42 -0700 Subject: [PATCH 1/3] Fix https://github.com/bmad-sim/bmad-ecosystem/issues/2049 --- tao/code/tao_pipe_cmd.f90 | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tao/code/tao_pipe_cmd.f90 b/tao/code/tao_pipe_cmd.f90 index 20082b131..7a4e26861 100644 --- a/tao/code/tao_pipe_cmd.f90 +++ b/tao/code/tao_pipe_cmd.f90 @@ -9531,7 +9531,7 @@ end subroutine write_this_floor recursive subroutine write_this_ele_floor(ele, loc, can_vary, suffix) type (ele_struct), target :: ele -type (ele_struct), pointer :: ele0 +type (ele_struct), pointer :: ele0, slave_ele integer n, ie, loc logical can_vary @@ -9548,6 +9548,31 @@ recursive subroutine write_this_ele_floor(ele, loc, can_vary, suffix) return endif +! A girder_lord has no place on the s-axis, so beginning/end refer to the +! upstream end of the first slave and the downstream end of the last slave. + +if (ele%lord_status == girder_lord$) then + select case (loc) + case (1) + slave_ele => pointer_to_slave(ele, 1) + if (slave_ele%ix_ele == 0) then + ele0 => slave_ele + else + ele0 => pointer_to_next_ele(slave_ele, -1) + endif + call write_this_floor(ele0%floor, 'Reference' // suffix, can_vary) + call write_this_floor(ele_geometry_with_misalignments (slave_ele, 0.0_rp), 'Actual' // suffix, .false.) + case (2) + call write_this_floor(ele%floor, 'Reference' // suffix, can_vary) + call write_this_floor(ele_geometry_with_misalignments (ele), 'Actual' // suffix, .false.) + case (3) + slave_ele => pointer_to_slave(ele, ele%n_slave) + call write_this_floor(slave_ele%floor, 'Reference' // suffix, can_vary) + call write_this_floor(ele_geometry_with_misalignments (slave_ele), 'Actual' // suffix, .false.) + end select + return +endif + ! if (ele%ix_ele == 0) then From efc77a387836cc3848b3b1687593c1c40d945fb9 Mon Sep 17 00:00:00 2001 From: Christopher Mayes <31023527+ChristopherMayes@users.noreply.github.com> Date: Mon, 8 Jun 2026 14:11:30 -0700 Subject: [PATCH 2/3] Add test --- regression_tests/pytao/girder_floor/lat.bmad | 18 ++++++ .../pytao/girder_floor/test_girder_floor.py | 59 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 regression_tests/pytao/girder_floor/lat.bmad create mode 100644 regression_tests/pytao/girder_floor/test_girder_floor.py diff --git a/regression_tests/pytao/girder_floor/lat.bmad b/regression_tests/pytao/girder_floor/lat.bmad new file mode 100644 index 000000000..5a6756a59 --- /dev/null +++ b/regression_tests/pytao/girder_floor/lat.bmad @@ -0,0 +1,18 @@ +! Regression test lattice for "pipe ele:floor" on a girder_lord. +! +! Original error: `pipe ele:floor g1 beginning` crashed Tao with SIGSEGV +! because tao_pipe_cmd dereferenced a null pointer when the element was a +! girder_lord (pointer_to_next_ele returns null for non-super_lord lords). + +beginning[beta_a] = 1 +beginning[beta_b] = 1 +beginning[e_tot] = 100e6 +parameter[geometry] = open + +p1: pipe, L = 1 +p2: pipe, L = 2 +g1: girder = {p1, p2} + +lat: line = (p1, p2) + +use, lat diff --git a/regression_tests/pytao/girder_floor/test_girder_floor.py b/regression_tests/pytao/girder_floor/test_girder_floor.py new file mode 100644 index 000000000..10cd8ec57 --- /dev/null +++ b/regression_tests/pytao/girder_floor/test_girder_floor.py @@ -0,0 +1,59 @@ +"""Regression tests for `pipe ele:floor` on girder_lord elements.""" + +from pathlib import Path + +import numpy as np +import pytest +from pytao import SubprocessTao + + +LAT_PATH = Path(__file__).parent / "lat.bmad" + + +@pytest.fixture(scope="module") +def tao(): + assert LAT_PATH.is_file(), f"Lattice file not found: {LAT_PATH}" + with SubprocessTao(lattice_file=str(LAT_PATH), noplot=True) as t: + yield t + + +def test_girder_floor_beginning_does_not_crash(tao): + """Regression test: `pipe ele:floor beginning` used to segfault. + + For a girder_lord the previous implementation called + `pointer_to_next_ele(ele, -1)`, which returns null for any lord that is + not a super_lord, and then dereferenced the result. + """ + result = tao.ele_floor("g1", where="beginning") + # Floor position of the upstream end of the first slave (p1) is the + # lattice origin. + np.testing.assert_allclose(result["Reference"], [0, 0, 0, 0, 0, 0], atol=1e-12) + np.testing.assert_allclose(result["Actual"], [0, 0, 0, 0, 0, 0], atol=1e-12) + + +def test_girder_floor_end_matches_last_slave(tao): + """`pipe ele:floor end` returns the downstream end of the last slave.""" + girder_end = tao.ele_floor("g1", where="end") + last_slave_end = tao.ele_floor("p2", where="end") + np.testing.assert_allclose(girder_end["Reference"], last_slave_end["Reference"], atol=1e-12) + np.testing.assert_allclose(girder_end["Actual"], last_slave_end["Actual"], atol=1e-12) + # Total length p1+p2 = 3 m, lattice is along z. + np.testing.assert_allclose(girder_end["Reference"], [0, 0, 3, 0, 0, 0], atol=1e-12) + + +def test_girder_floor_beginning_matches_first_slave(tao): + """`pipe ele:floor beginning` returns the upstream end of the first slave.""" + girder_begin = tao.ele_floor("g1", where="beginning") + first_slave_begin = tao.ele_floor("p1", where="beginning") + np.testing.assert_allclose( + girder_begin["Reference"], first_slave_begin["Reference"], atol=1e-12 + ) + np.testing.assert_allclose(girder_begin["Actual"], first_slave_begin["Actual"], atol=1e-12) + + +def test_girder_floor_center(tao): + """`pipe ele:floor center` returns the girder's own floor position.""" + result = tao.ele_floor("g1", where="center") + # Center of a girder is the midpoint of the slave span: z = (0 + 3) / 2 = 1.5. + np.testing.assert_allclose(result["Reference"], [0, 0, 1.5, 0, 0, 0], atol=1e-12) + np.testing.assert_allclose(result["Actual"], [0, 0, 1.5, 0, 0, 0], atol=1e-12) From d092d7caeb6c9771a5d4ca74b845d9933e3a08c2 Mon Sep 17 00:00:00 2001 From: Christopher Mayes <31023527+ChristopherMayes@users.noreply.github.com> Date: Tue, 9 Jun 2026 21:00:32 -0700 Subject: [PATCH 3/3] Previous had a bug with multipass. Fix and add a test --- .../pytao/girder_floor/lat_multipass.bmad | 23 +++++++++++ .../pytao/girder_floor/test_girder_floor.py | 23 +++++++++++ tao/code/tao_pipe_cmd.f90 | 41 ++++++++++--------- 3 files changed, 68 insertions(+), 19 deletions(-) create mode 100644 regression_tests/pytao/girder_floor/lat_multipass.bmad diff --git a/regression_tests/pytao/girder_floor/lat_multipass.bmad b/regression_tests/pytao/girder_floor/lat_multipass.bmad new file mode 100644 index 000000000..90caae655 --- /dev/null +++ b/regression_tests/pytao/girder_floor/lat_multipass.bmad @@ -0,0 +1,23 @@ +! Regression test lattice for "pipe ele:floor" on a girder_lord whose slaves +! are multipass_lords. The original crash for this configuration was a +! follow-on to issue #2049: pointer_to_slave(girder, 1) returns the +! multipass_lord (which lives outside the tracking part of the lattice), +! and pointer_to_next_ele on a lord returns null. The fix uses +! find_element_ends to walk through nested lord chains. + +beginning[beta_a] = 1 +beginning[beta_b] = 1 +beginning[e_tot] = 100e6 +parameter[geometry] = open + +p1: pipe, L = 1 +p2: pipe, L = 2 + +! Multipass line: p1 and p2 are turned into multipass_lord elements. +mp_line: line[multipass] = (p1, p2) + +g1: girder = {p1, p2} + +lat: line = (mp_line, mp_line) + +use, lat diff --git a/regression_tests/pytao/girder_floor/test_girder_floor.py b/regression_tests/pytao/girder_floor/test_girder_floor.py index 10cd8ec57..c0c125339 100644 --- a/regression_tests/pytao/girder_floor/test_girder_floor.py +++ b/regression_tests/pytao/girder_floor/test_girder_floor.py @@ -8,6 +8,7 @@ LAT_PATH = Path(__file__).parent / "lat.bmad" +LAT_MULTIPASS_PATH = Path(__file__).parent / "lat_multipass.bmad" @pytest.fixture(scope="module") @@ -17,6 +18,13 @@ def tao(): yield t +@pytest.fixture(scope="module") +def tao_multipass(): + assert LAT_MULTIPASS_PATH.is_file(), f"Lattice file not found: {LAT_MULTIPASS_PATH}" + with SubprocessTao(lattice_file=str(LAT_MULTIPASS_PATH), noplot=True) as t: + yield t + + def test_girder_floor_beginning_does_not_crash(tao): """Regression test: `pipe ele:floor beginning` used to segfault. @@ -57,3 +65,18 @@ def test_girder_floor_center(tao): # Center of a girder is the midpoint of the slave span: z = (0 + 3) / 2 = 1.5. np.testing.assert_allclose(result["Reference"], [0, 0, 1.5, 0, 0, 0], atol=1e-12) np.testing.assert_allclose(result["Actual"], [0, 0, 1.5, 0, 0, 0], atol=1e-12) + + +def test_girder_floor_multipass_does_not_crash(tao_multipass): + """Regression test: `pipe ele:floor beginning` used to segfault + when the girder's slaves are multipass_lord elements. + + pointer_to_slave(girder, 1) returns the multipass_lord (which is not in + the tracking part of the lattice), so pointer_to_next_ele returned null. + The fix descends through nested lord chains via find_element_ends. + """ + begin = tao_multipass.ele_floor("g1", where="beginning") + end = tao_multipass.ele_floor("g1", where="end") + # First pass of the multipass line is at z in [0, 3]. + np.testing.assert_allclose(begin["Reference"], [0, 0, 0, 0, 0, 0], atol=1e-12) + np.testing.assert_allclose(end["Reference"], [0, 0, 3, 0, 0, 0], atol=1e-12) diff --git a/tao/code/tao_pipe_cmd.f90 b/tao/code/tao_pipe_cmd.f90 index 7a4e26861..821cbc21d 100644 --- a/tao/code/tao_pipe_cmd.f90 +++ b/tao/code/tao_pipe_cmd.f90 @@ -9531,7 +9531,7 @@ end subroutine write_this_floor recursive subroutine write_this_ele_floor(ele, loc, can_vary, suffix) type (ele_struct), target :: ele -type (ele_struct), pointer :: ele0, slave_ele +type (ele_struct), pointer :: ele0, ele1, ele2 integer n, ie, loc logical can_vary @@ -9549,26 +9549,29 @@ recursive subroutine write_this_ele_floor(ele, loc, can_vary, suffix) endif ! A girder_lord has no place on the s-axis, so beginning/end refer to the -! upstream end of the first slave and the downstream end of the last slave. +! upstream end of the first tracking slave and the downstream end of the last +! tracking slave. find_element_ends walks through nested lord chains +! (multipass_lord, super_lord, nested girders) to reach a tracking element. if (ele%lord_status == girder_lord$) then + call find_element_ends(ele, ele1, ele2) + if (.not. associated(ele1) .or. .not. associated(ele2)) then + call invalid ('UNABLE TO FIND ENDS OF GIRDER: ' // ele%name) + return + endif select case (loc) - case (1) - slave_ele => pointer_to_slave(ele, 1) - if (slave_ele%ix_ele == 0) then - ele0 => slave_ele - else - ele0 => pointer_to_next_ele(slave_ele, -1) - endif - call write_this_floor(ele0%floor, 'Reference' // suffix, can_vary) - call write_this_floor(ele_geometry_with_misalignments (slave_ele, 0.0_rp), 'Actual' // suffix, .false.) - case (2) + case (1) ! beginning + ! ele1 is the element immediately upstream of the first tracking slave, + ! so ele1%floor is the upstream end of the first tracking slave. + call write_this_floor(ele1%floor, 'Reference' // suffix, can_vary) + ele0 => pointer_to_next_ele(ele1, +1) + call write_this_floor(ele_geometry_with_misalignments (ele0, 0.0_rp), 'Actual' // suffix, .false.) + case (2) ! center call write_this_floor(ele%floor, 'Reference' // suffix, can_vary) call write_this_floor(ele_geometry_with_misalignments (ele), 'Actual' // suffix, .false.) - case (3) - slave_ele => pointer_to_slave(ele, ele%n_slave) - call write_this_floor(slave_ele%floor, 'Reference' // suffix, can_vary) - call write_this_floor(ele_geometry_with_misalignments (slave_ele), 'Actual' // suffix, .false.) + case (3) ! end + call write_this_floor(ele2%floor, 'Reference' // suffix, can_vary) + call write_this_floor(ele_geometry_with_misalignments (ele2), 'Actual' // suffix, .false.) end select return endif @@ -9582,14 +9585,14 @@ recursive subroutine write_this_ele_floor(ele, loc, can_vary, suffix) endif select case (loc) -case (1) +case (1) ! beginning call write_this_floor(ele0%floor, 'Reference' // suffix, can_vary) call write_this_floor(ele_geometry_with_misalignments (ele, 0.0_rp), 'Actual' // suffix, .false.) -case (2) +case (2) ! center call ele_geometry(ele0%floor, ele, floor, 0.5_rp) call write_this_floor(floor, 'Reference' // suffix, can_vary) call write_this_floor(ele_geometry_with_misalignments (ele, 0.5_rp), 'Actual' // suffix, .false.) -case (3) +case (3) ! end call write_this_floor(ele%floor, 'Reference' // suffix, can_vary) call write_this_floor(ele_geometry_with_misalignments (ele), 'Actual' // suffix, .false.) end select