Diamond 2 conversion #145

ptsOSL · 2025-09-15T14:27:19Z

Update all diamond 1 ringmodes using the version of load_mml.m from main. This is because there were some major changes to DIAD, VMX and SRI0913_MOGA ~3 years ago which hadnt been applied to our configs.
Update load_mml.m and load_unitconv.m for the diamond 2 ring and ao.
load_mml.m and load_unitconv.m should now work for both diamond 1 and diamond 2 ringmodes.
Remove redundant "name" column from elements.csv file generation and from the pytac code which loads this csv.
Added multipoles to rigidity_families dictionary
Regenerated all ringmodes, including the addition of the D2 ringmode, 48, using the updaded matlab scripts.
Minor refactor to load_mml to reduce the number of exceptions required by doing lookups using FamName instead of Class
Update CSVs to use correct PV names for BBVMXL and BBVMXS

These mismatch currently, THERING uses 49 while the ao uses 48. They should match ideally but this is still WIP

Octupoles and Dipoles do not currently have a group FamilyName, this may be added in the future?

There are now multiple elements which are linked to LI-RF-MOSC-01 where previously there was just one, so we need to add the unit conversion for all of them

Instead of hardcoding, get it from the ao

Elements have both a FamName and a Class, with the FamName typically being a specific instance of a Class. Class is also referred to as type in mml. Because of differenes between the DI ao and the DII ao, we need to have awkward conversions and special cases for different element families. eg in DI lattice, there is only 1 FamName in the Bend Class, but in DII there are many and the Bend Class is used in the lattice but not in the ao. We also have to handle special cases for combined Sexts/Octs and Steerers and a special case for combined bend and bend trims (BBVMXS & BBVMXL). This code is not pretty and could do with a refactor, but does currently provide the same configuration for I04 as the old script, as well as creating a working DII configuration.

This is being done prior to major changes to the generation scripts for the D2 ringmodes

Currently these are just octupoles. Support the b3 field and also combined hstr/vstr/multipole elements

Old modes are generated using the updated scripts, the only changes is the removal of the redundant name column from elements.csv

The name was only ever equal to and so this was already guarenteed to be None based off the load_mml.m script

ptsOSL · 2025-09-15T14:56:03Z

FAILED tests/test_load.py::test_available_ringmodes - AssertionError: assert {'DIADTHz', 'DIADSP', 'VMXTHz', 'I04SP', 'SRI0913_MOGA', 'I04', 'DIAD', 'VMX', 'VMXSP', 'I04THz', '4...
FAILED tests/test_machine.py::test_load_lattice_using_default_dir - assert 2192 == 2142
FAILED tests/test_machine.py::test_load_lattice[vmx_ring-VMX-2142-561.571] - assert 2192 == 2142
FAILED tests/test_machine.py::test_load_lattice[diad_ring-DIAD-2144-561.571] - assert 2194 == 2144
FAILED tests/test_machine.py::test_load_drift_elements[vmx_ring-1308] - assert 1343 == 1308
FAILED tests/test_machine.py::test_load_drift_elements[diad_ring-1311] - assert 1346 == 1311
FAILED tests/test_machine.py::test_quad_unitconv_with_different_limits[747--1.9457] - pytac.exceptions.UnitsException: PchipUnitConv 5: None of conversion results [2456.125890667614, -199.09305292447007, -1438...
FAILED tests/test_machine.py::test_quad_unitconv_with_different_limits[1135--1.9864] - pytac.exceptions.FieldException: <Element index 1135, length 0.4218 m, cell 12, families drift, d3a>: No unit conversion op...

A bug in load_mml was causing quadrupole elements to be configured with the wrong PV names. This meant that when doing tunefb, we were writing to the wrong PVs and wrong elements, causing tunefb to go wrong. I have changed load_mml so that it looks up the element indexes in the quadrupole family list of pvs, rather than in the combined quadrupole list of pvs.

src/pytac/data/utils/load_unitconv.m

src/pytac/data/utils/load_mml.m

src/pytac/data/utils/load_unitconv.m

src/pytac/data/utils/load_mml.m

DIAD was regenerated and changed length due to modifications made in mml 3 years ago

The MML lattice has been altered,some RF cavities have been moved and AT markers added

By default we now index using elem.FamName instead of Class. The only exception is for FamNames defined in TYPE_MAP which are remapped to different lookup keys which are needed to lookup data from the ao. lookup_key is also now only defined in one place. This change has uncovered that we were previously using the wrong PV names for BBVMXL elements, these were previously being treated as regular BB elements. This wont actually change anything in Virtac as we treat all 'BEND' magnets the same using the same PV. BBVMXL and BBVMXS are not accurately simulated.

codecov · 2025-09-24T10:57:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.11%. Comparing base (27571aa) to head (97f9e6a).
⚠️ Report is 31 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #145   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files          12       12           
  Lines         788      788           
=======================================
  Hits          781      781           
  Misses          7        7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/pytac/data/utils/load_mml.m

src/pytac/data/utils/load_unitconv.m

src/pytac/load_csv.py

Currently our csv files dont specify names, but in the future we might want to. So I have added back in support for names specified in future csv files.

ptsOSL · 2025-09-26T16:07:33Z

I need to update the tests to remove references to the old ringmodes. And then message tobyn and then this is ready to merge

It would crash if the csv had no name column

The tests now test against I04 and DIAD instead of VMX and DIAD. I also factored out a couple of constants from the test files

ptsOSL · 2025-10-02T10:34:30Z

Fixed tests

MJGaughran

I think this is fine. A more descriptive name for the '48' ring mode would be good, but this shouldn't impact anything if we decide to change it later on.

ptsOSL added 15 commits September 15, 2025 15:38

Temporarily remove check for ringtype

c0452b4

These mismatch currently, THERING uses 49 while the ao uses 48. They should match ideally but this is still WIP

Remove empty name column from elements.csv

de338e1

Beam energy is now read from THERING

c330f17

Update TYPEMAP with new FamNames from ao

895b4a8

Octupoles and Dipoles do not currently have a group FamilyName, this may be added in the future?

Handle multiple elements of family RF

3856f64

There are now multiple elements which are linked to LI-RF-MOSC-01 where previously there was just one, so we need to add the unit conversion for all of them

Get energy from THERING instead of hardcoding it

81ebeb2

Get master oscillator PV properly

58fd70e

Instead of hardcoding, get it from the ao

Regenerate all D1 ringmodes

b6c92ea

This is being done prior to major changes to the generation scripts for the D2 ringmodes

Re-enable checking for correct ringmode

80a89f3

Support multipoles

f649362

Currently these are just octupoles. Support the b3 field and also combined hstr/vstr/multipole elements

Support octupoles in unit conversions

561dbf2

Generate ringmode 48 and regenerate old modes

2e16c58

Old modes are generated using the updated scripts, the only changes is the removal of the redundant name column from elements.csv

Remove getting 'name' from the elements.csv file

d517e76

The name was only ever equal to and so this was already guarenteed to be None based off the load_mml.m script

Support getting the rigidity from multipoles

17ad36f

ptsOSL force-pushed the diamond2-conversion branch from 8171377 to 17ad36f Compare September 15, 2025 14:40

ptsOSL changed the title ~~Draft: Diamond 2 conversion~~ Diamond 2 conversion Sep 15, 2025

ptsOSL requested review from MJGaughran and T-Nicholls September 15, 2025 14:45

ptsOSL added 2 commits September 16, 2025 16:55

Regenerate csv with correct OX0 PV names

16fb10e

MJGaughran reviewed Sep 18, 2025

View reviewed changes

T-Nicholls reviewed Sep 19, 2025

View reviewed changes

src/pytac/data/utils/load_mml.m Show resolved Hide resolved

ptsOSL added 4 commits September 22, 2025 12:33

Fix tests for ringmode 48 and updated DIAD

8660404

DIAD was regenerated and changed length due to modifications made in mml 3 years ago

Update ringmode 48 csv data

e7df633

The MML lattice has been altered,some RF cavities have been moved and AT markers added

Write unit conversions data for both Oct families

61203cc

T-Nicholls reviewed Sep 25, 2025

View reviewed changes

src/pytac/data/utils/load_mml.m Show resolved Hide resolved

src/pytac/data/utils/load_mml.m Show resolved Hide resolved

src/pytac/data/utils/load_unitconv.m Outdated Show resolved Hide resolved

src/pytac/load_csv.py Outdated Show resolved Hide resolved

ptsOSL added 3 commits September 26, 2025 15:07

Remove unnecessary if statement for RF data

0cdcae0

Add back in support for name col in csv file

d2d41fe

Currently our csv files dont specify names, but in the future we might want to. So I have added back in support for names specified in future csv files.

Deprecate old ringmodes

ff1750e

ptsOSL added 3 commits October 1, 2025 08:55

Fix bug when getting name from csv files

7f891c1

It would crash if the csv had no name column

Switch tests from VMX to I04

276a0ba

The tests now test against I04 and DIAD instead of VMX and DIAD. I also factored out a couple of constants from the test files

Run Ruff format to fix ci failures

7caa82e

ptsOSL requested a review from T-Nicholls October 1, 2025 09:23

MJGaughran previously approved these changes Oct 5, 2025

View reviewed changes

Make getting name from csv file cleaner

97f9e6a

ptsOSL dismissed MJGaughran’s stale review via 97f9e6a October 6, 2025 09:32

ptsOSL merged commit 08c86ad into main Oct 6, 2025
16 checks passed

ptsOSL deleted the diamond2-conversion branch October 9, 2025 09:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Diamond 2 conversion #145

Diamond 2 conversion #145

Uh oh!

ptsOSL commented Sep 15, 2025 •

edited

Loading

Uh oh!

ptsOSL commented Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ptsOSL commented Sep 26, 2025

Uh oh!

ptsOSL commented Oct 2, 2025

Uh oh!

MJGaughran left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Diamond 2 conversion #145

Diamond 2 conversion #145

Uh oh!

Conversation

ptsOSL commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptsOSL commented Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ptsOSL commented Sep 26, 2025

Uh oh!

ptsOSL commented Oct 2, 2025

Uh oh!

MJGaughran left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ptsOSL commented Sep 15, 2025 •

edited

Loading

codecov bot commented Sep 24, 2025 •

edited

Loading