Skip to content

Commit 8aaf7ed

Browse files
jgray-19fsoubelet
jgray-19
and
fsoubelet
authored
Remove backwards compatibility for Drive (#556)
* Add amplitude unit handling and backward compatibility tests - Introduced HEADER_AMPLITUDE_UNIT and AMPLITUDE_UNIT_VALUE constants for amplitude unit management. - Updated the _compute_headers function to include the amplitude unit in headers. - Modified InputFiles class to handle old and new amplitude formats correctly. - Removed outdated scaling factors in tests and added new tests for backward compatibility. * Update changelog for v0.25.0 and set version to 0.25.0 * Change files to address comments --------- Co-authored-by: Felix Soubelet <[email protected]>
1 parent 08d2548 commit 8aaf7ed

File tree

8 files changed

+181
-74
lines changed

8 files changed

+181
-74
lines changed

‎CHANGELOG.md‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,20 @@
11
# OMC3 Changelog
22

3-
#### 2025-10-TBD - v0.24.6 - _jgray_, _jdilly_, _fsoubelet_
3+
#### 2025-11-TBD - v0.25.0 - _jgray_, _jdilly_, _fsoubelet_
4+
5+
- Added:
6+
- Tests for amplitude unit handling and backward compatibility.
7+
- Allow passing of TbTData objects to hole-in-one directly
8+
9+
- Changes:
10+
- All new amplitude data files now include a header `AMPLITUDE_UNIT` specifying the unit of amplitude values.
11+
- Before, we doubled the amplitude values read from files and divided by 2 when writing to maintain compatibility with old files. This is now removed, and no longer the case.
12+
13+
14+
#### 2025-10-29 - v0.24.6 - _jgray_, _jdilly_, _fsoubelet_
15+
16+
- Breaking change:
17+
- Now requires `turn_by_turn>=1.0.0` due to new `meta` parameter.
418

519
- Added:
620
- Throw an Error when Tune Tolerance is too Large, i.e. driven tunes are inside the tolerance of the natural tunes.

‎omc3/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
__title__ = "omc3"
1212
__description__ = "An accelerator physics tools package for the OMC team at CERN."
1313
__url__ = "https://github.com/pylhc/omc3"
14-
__version__ = "0.24.6"
14+
__version__ = "0.25.0"
1515
__author__ = "pylhc"
1616
__author_email__ = "[email protected]"
1717
__license__ = "MIT"

‎omc3/harpy/constants.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@
4545
COL_COEFFS: str = "COEFFS"
4646
COL_FREQS: str = "FREQS"
4747
COL_TIME: str = "TIME"
48+
49+
# Header Keys -----------------------------------------------------------------
50+
MAINLINE_UNIT: str = "MAINLINE_UNIT"

‎omc3/harpy/handler.py‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
FILE_AMPS_EXT,
4343
FILE_FREQS_EXT,
4444
FILE_LIN_EXT,
45+
MAINLINE_UNIT,
4546
)
4647
from omc3.utils import logging_tools
4748
from omc3.utils.contexts import timeit
@@ -251,6 +252,7 @@ def _compute_headers(panda: pd.DataFrame, date: None | pd.Timestamp = None) -> d
251252
) # TODO: not really the RMS?
252253
if date:
253254
headers[COL_TIME] = date.strftime(formats.TIME)
255+
headers[MAINLINE_UNIT] = "m"
254256
return headers
255257

256258

@@ -283,18 +285,12 @@ def _write_lin_tfs(output_path_without_suffix: Path, plane: str, lin_frame: pd.D
283285

284286
def _rescale_amps_to_main_line_and_compute_noise(df: pd.DataFrame, plane: str) -> pd.DataFrame:
285287
"""
286-
TODO follows non-transpararent convention
287-
TODO the consequent analysis has to be changed if removed
288+
Rescale secondary amplitudes to main line amplitude and compute noise-related errors.
288289
"""
289290
cols = [col for col in df.columns if col.startswith(COL_AMP)]
290291
cols.remove(f"{COL_AMP}{plane}")
291292
df[cols] = df[cols].div(df[f"{COL_AMP}{plane}"], axis="index")
292293
amps = df[f"{COL_AMP}{plane}"].to_numpy()
293-
# Division by two for backwards compatibility with Drive, i.e. the unit is [2mm] (03/2019)
294-
# TODO later remove (05/2019)
295-
df[f"{COL_AMP}{plane}"] = amps / 2
296-
if f"{COL_NATAMP}{plane}" in df.columns:
297-
df[f"{COL_NATAMP}{plane}"] = df[f"{COL_NATAMP}{plane}"].to_numpy() / 2
298294

299295
if df[COL_NOISE].max() == 0.0:
300296
return df # Do not calculate errors when no noise was calculated

‎omc3/optics_measurements/data_models.py‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
55
Models used in optics measurements to store and pass around data.
66
"""
7+
78
from __future__ import annotations
89

910
from pathlib import Path
@@ -14,6 +15,7 @@
1415
import tfs
1516

1617
from omc3.definitions.constants import PLANES
18+
from omc3.harpy.constants import MAINLINE_UNIT
1719
from omc3.optics_measurements import dpp, iforest
1820
from omc3.optics_measurements.constants import (
1921
AMPLITUDE,
@@ -31,6 +33,7 @@
3133

3234
LOGGER = logging_tools.get_logger(__name__)
3335

36+
3437
class InputFiles(dict):
3538
"""
3639
Stores the input files, provides methods to gather quantity specific data
@@ -63,13 +66,27 @@ def __init__(self, files_to_analyse: Sequence[str|Path|tfs.TfsDataFrame], optics
6366
self[plane] = dpp.append_dpp(self[plane], dpp.arrange_dpps(dpp_values))
6467
self[plane] = dpp.append_amp_dpp(self[plane], amp_dpp_values)
6568

66-
@staticmethod # TODO later remove
67-
def _repair_backwards_compatible_frame(df, plane: str):
69+
@staticmethod
70+
def _repair_backwards_compatible_frame(
71+
df: pd.DataFrame | tfs.TfsDataFrame, plane: str
72+
) -> pd.DataFrame | tfs.TfsDataFrame:
6873
"""
69-
Multiplies unscaled amplitudes by 2 to get from complex amplitudes to the real ones.
70-
This is for backwards compatibility with Drive,
71-
i.e. harpy has this
74+
Multiplies unscaled amplitudes by 2 for old files without the AMPLITUDE_UNIT header.
75+
This is for backwards compatibility with Drive.
76+
77+
New harpy files (with AMPLITUDE_UNIT header set to 'm') don't need this correction
78+
as they already output amplitudes in the correct unit.
7279
"""
80+
# Check if the file has the AMPLITUDE_UNIT header since v0.25.0
81+
# If it does, no correction is needed
82+
if (unit := getattr(df, "headers", {}).get(MAINLINE_UNIT)) is not None:
83+
LOGGER.info(f"Detected amplitude unit '{unit}' for plane {plane}.")
84+
if unit != "m":
85+
# Potentially in the future, we could add conversion here if needed (jgray 10/2025)
86+
raise ValueError(f"Unexpected amplitude unit '{unit}' in file for plane {plane}.")
87+
return df
88+
89+
# Old files without the header need the correction (multiplication by 2)
7390
df[f"AMP{plane}"] = df.loc[:, f"AMP{plane}"].to_numpy() * 2
7491
if f"NATAMP{plane}" in df.columns:
7592
df[f"NATAMP{plane}"] = df.loc[:, f"NATAMP{plane}"].to_numpy() * 2

‎tests/accuracy/test_harpy.py‎

Lines changed: 16 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -178,33 +178,17 @@ def test_freekick_harpy(_test_file: Path, _model_file: Path):
178178
for plane in PLANES:
179179
# Main and secondary frequencies
180180
assert (
181-
_rms(
182-
_diff(
183-
linfiles[plane].loc[:, f"TUNE{plane}"].to_numpy(),
184-
model_df.loc[:, f"TUNE{plane}"].to_numpy(),
185-
)
186-
)
181+
_rms(_diff(linfiles[plane].loc[:, f"TUNE{plane}"], model_df.loc[:, f"TUNE{plane}"]))
187182
< LIMITS["F1"]
188183
)
189184
# Main and secondary amplitudes
190-
# TODO remove factor 2 - only for backwards compatibility with Drive
191185
assert (
192-
_rms(
193-
_rel_diff(
194-
linfiles[plane].loc[:, f"AMP{plane}"].to_numpy() * 2,
195-
model_df.loc[:, f"AMP{plane}"].to_numpy(),
196-
)
197-
)
186+
_rms(_rel_diff(linfiles[plane].loc[:, f"AMP{plane}"], model_df.loc[:, f"AMP{plane}"]))
198187
< LIMITS["A1"]
199188
)
200189
# Main and secondary phases
201190
assert (
202-
_rms(
203-
_angle_diff(
204-
linfiles[plane].loc[:, f"MU{plane}"].to_numpy(),
205-
model_df.loc[:, f"MU{plane}"].to_numpy(),
206-
)
207-
)
191+
_rms(_angle_diff(linfiles[plane].loc[:, f"MU{plane}"], model_df.loc[:, f"MU{plane}"]))
208192
< LIMITS["P1"]
209193
)
210194

@@ -229,79 +213,55 @@ def test_harpy_3d(_test_file: Path, _model_file: Path):
229213
linfiles = {"X": tfs.read(f"{_test_file}.linx"), "Y": tfs.read(f"{_test_file}.liny")}
230214
model_df = tfs.read(_model_file)
231215
_assert_spectra(linfiles, model_df)
232-
assert _rms(_diff(linfiles["X"].loc[:, "TUNEZ"].to_numpy(), TUNEZ)) < LIMITS["F2"]
216+
assert _rms(_diff(linfiles["X"].loc[:, "TUNEZ"], TUNEZ)) < LIMITS["F2"]
233217
assert (
234-
_rms(
235-
_rel_diff(
236-
linfiles["X"].loc[:, "AMPZ"].to_numpy()
237-
* linfiles["X"].loc[:, "AMPX"].to_numpy()
238-
* 2,
239-
AMPZ * BASEAMP,
240-
)
241-
)
218+
_rms(_rel_diff(linfiles["X"].loc[:, "AMPZ"] * linfiles["X"].loc[:, "AMPX"], AMPZ * BASEAMP))
242219
< LIMITS["A2"]
243220
)
244-
assert _rms(_angle_diff(linfiles["X"].loc[:, "MUZ"].to_numpy(), MUZ)) < LIMITS["P2"]
221+
assert _rms(_angle_diff(linfiles["X"].loc[:, "MUZ"], MUZ)) < LIMITS["P2"]
245222

246223

247224
def _assert_spectra(linfiles: dict[str, tfs.TfsDataFrame], model_df: tfs.TfsDataFrame):
248225
for plane in PLANES:
249226
# main and secondary frequencies
250227
assert (
251-
_rms(
252-
_diff(
253-
linfiles[plane].loc[:, f"TUNE{plane}"].to_numpy(),
254-
model_df.loc[:, f"TUNE{plane}"].to_numpy(),
255-
)
256-
)
228+
_rms(_diff(linfiles[plane].loc[:, f"TUNE{plane}"], model_df.loc[:, f"TUNE{plane}"]))
257229
< LIMITS["F1"]
258230
)
259231
assert (
260232
_rms(
261233
_diff(
262-
linfiles[plane].loc[:, f"FREQ{_couple(plane)}"].to_numpy(),
263-
model_df.loc[:, f"TUNE{_other(plane)}"].to_numpy(),
234+
linfiles[plane].loc[:, f"FREQ{_couple(plane)}"],
235+
model_df.loc[:, f"TUNE{_other(plane)}"],
264236
)
265237
)
266238
< LIMITS["F2"]
267239
)
268240
# main and secondary amplitudes
269-
# TODO remove factor 2 - only for backwards compatibility with Drive
270241
assert (
271-
_rms(
272-
_rel_diff(
273-
linfiles[plane].loc[:, f"AMP{plane}"].to_numpy() * 2,
274-
model_df.loc[:, f"AMP{plane}"].to_numpy(),
275-
)
276-
)
242+
_rms(_rel_diff(linfiles[plane].loc[:, f"AMP{plane}"], model_df.loc[:, f"AMP{plane}"]))
277243
< LIMITS["A1"]
278244
)
279245
assert (
280246
_rms(
281247
_rel_diff(
282-
linfiles[plane].loc[:, f"AMP{_couple(plane)}"].to_numpy()
283-
* linfiles[plane].loc[:, f"AMP{plane}"].to_numpy()
284-
* 2,
285-
COUPLING * model_df.loc[:, f"AMP{_other(plane)}"].to_numpy(),
248+
linfiles[plane].loc[:, f"AMP{_couple(plane)}"]
249+
* linfiles[plane].loc[:, f"AMP{plane}"],
250+
COUPLING * model_df.loc[:, f"AMP{_other(plane)}"],
286251
)
287252
)
288253
< LIMITS["A2"]
289254
)
290255
# main and secondary phases
291256
assert (
292-
_rms(
293-
_angle_diff(
294-
linfiles[plane].loc[:, f"MU{plane}"].to_numpy(),
295-
model_df.loc[:, f"MU{plane}"].to_numpy(),
296-
)
297-
)
257+
_rms(_angle_diff(linfiles[plane].loc[:, f"MU{plane}"], model_df.loc[:, f"MU{plane}"]))
298258
< LIMITS["P1"]
299259
)
300260
assert (
301261
_rms(
302262
_angle_diff(
303-
linfiles[plane].loc[:, f"PHASE{_couple(plane)}"].to_numpy(),
304-
model_df.loc[:, f"MU{_other(plane)}"].to_numpy(),
263+
linfiles[plane].loc[:, f"PHASE{_couple(plane)}"],
264+
model_df.loc[:, f"MU{_other(plane)}"],
305265
)
306266
)
307267
< LIMITS["P2"]

‎tests/accuracy/twiss_to_lin.py‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
COL_S,
3737
COL_TIME,
3838
COL_TUNE,
39+
MAINLINE_UNIT,
3940
)
4041
from omc3.optics_measurements.constants import NAT_TUNE, TUNE
4142

@@ -165,10 +166,6 @@ def generate_lin_files(model, tune, nattune, motion="_d", dpp=0.0, beam_directio
165166
)
166167
) + COUPLING * noise_freq_domain * np.random.randn(nbpms)
167168

168-
# backwards compatibility with drive TODO remove
169-
lin[f"{COL_AMP}{plane}"] = lin.loc[:, f"{COL_AMP}{plane}"].to_numpy() / 2
170-
lin[f"{COL_NATAMP}{plane}"] = lin.loc[:, f"{COL_NATAMP}{plane}"].to_numpy() / 2
171-
172169
lins[plane] = tfs.TfsDataFrame(lin, headers=_get_header(tune, nattune, plane)).set_index(
173170
COL_NAME, drop=False
174171
)
@@ -192,4 +189,5 @@ def _get_header(tunes, nattunes, plane):
192189
f"{NAT_TUNE}{PLANE_TO_NUM[plane]}": nattunes[plane],
193190
f"{NAT_TUNE}{PLANE_TO_NUM[plane]}RMS": 1e-6,
194191
COL_TIME: datetime.now(timezone.utc).strftime(formats.TIME),
192+
MAINLINE_UNIT: "m",
195193
}

0 commit comments

Comments
 (0)