Skip to content

Commit 0b9578f

Browse files
sawenzelclaude
andcommitted
Fix the ITS ramp-up shift rounding and the Pylint errors on master
This fixes the millisecond rounding of the TF-aligned ITS ramp-up shift, updates the ramp-up test to the new signature and clears the remaining Pylint error. - The ramp-up is now rounded up to whole milliseconds before it is rounded up to whole TFs, so the shifted timestamp can no longer lie inside the ramp. - test_anchoring_rampup.py passes the TF length to shift_anchor_past_ITS_rampup and has a regression test for a ramp ending just below a TF boundary. - ccdb_cross_check in getCCDBTimeMachineTimestamp.py keeps the pinned object in a local variable, which Pylint can follow. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 parent d1a26b6 commit 0b9578f

3 files changed

Lines changed: 22 additions & 15 deletions

File tree

‎GRID/utils/getCCDBTimeMachineTimestamp.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,9 @@ def ccdb_cross_check(path, not_after, timeout=15):
8686

8787
as_of = [o for o in objects if o.get("Created", 0) <= not_after]
8888
newest = max(objects, key=lambda o: o.get("Created", 0))
89-
result = {"newest": newest, "as_of": None, "outdated": False}
90-
if as_of:
91-
result["as_of"] = max(as_of, key=lambda o: o.get("Created", 0))
92-
result["outdated"] = newest["Created"] > result["as_of"]["Created"]
93-
return result
89+
pinned = max(as_of, key=lambda o: o.get("Created", 0)) if as_of else None
90+
outdated = pinned is not None and newest["Created"] > pinned["Created"]
91+
return {"newest": newest, "as_of": pinned, "outdated": outdated}
9492

9593

9694
def describe_object(o):

‎MC/bin/o2dpg_sim_workflow_anchored.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,9 +336,9 @@ def shift_anchor_past_ITS_rampup(run_start, first_orbit, orbitsPerTF, ITS_rampup
336336
timestamp leaves a job at production offset 0 inside the ramp, where the ITS
337337
time-dead map masks every chip.
338338
"""
339-
# convert ITS_rampup to orbits multiple to orbitsPerTF
340-
rampupOrbits = ((milliseconds_to_orbits(ITS_rampup) + orbitsPerTF - 1) // orbitsPerTF ) * orbitsPerTF
341-
339+
# round the ramp-up up to whole milliseconds, then to a whole number of timeframes;
340+
# the truncated millisecond shift then still ends after the ramp and before the orbit
341+
rampupOrbits = -(-milliseconds_to_orbits(math.ceil(ITS_rampup)) // orbitsPerTF) * orbitsPerTF
342342
return run_start + int(rampupOrbits * LHCOrbitMUS / 1000.), first_orbit + rampupOrbits
343343

344344
def retrieve_MinBias_CTPScaler_Rate(raw_rate_at, finaltime, trig_eff_arg, NBunches, ColSystem, eCM, run_number = -1):

‎MC/bin/tests/test_anchoring_rampup.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
FIRST_ORBIT = 20505888
2222
SOR = 1778806732526
2323
ITS_RAMPUP_MS = 5000
24+
ORBITS_PER_TF = 128
2425
FIRST_ALIVE_ORBIT = 20539968
2526

2627

@@ -36,32 +37,40 @@ def test_orbits_from_milliseconds(self):
3637

3738
def test_both_coordinates_move(self):
3839
"""A ramp-up of a few seconds must move the orbit as well as the timestamp."""
39-
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ITS_RAMPUP_MS)
40-
self.assertEqual(start, SOR + ITS_RAMPUP_MS)
40+
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, ITS_RAMPUP_MS)
41+
self.assertGreaterEqual(start, SOR + ITS_RAMPUP_MS)
4142
self.assertGreater(orbit, FIRST_ORBIT)
43+
self.assertEqual((orbit - FIRST_ORBIT) % ORBITS_PER_TF, 0)
4244

4345
def test_nothing_moves_without_a_ramp(self):
44-
self.assertEqual(anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, 0),
46+
self.assertEqual(anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, 0),
4547
(SOR, FIRST_ORBIT))
4648

4749
def test_shifted_orbit_is_never_inside_the_ramp(self):
4850
"""The shifted orbit must sit at or after the shifted timestamp, never before."""
4951
for ramp_ms in (0, 1, 500, ITS_RAMPUP_MS, 30000):
50-
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ramp_ms)
52+
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, ramp_ms)
5153
time_of_orbit = SOR + (orbit - FIRST_ORBIT) * anchored.LHCOrbitMUS / 1000.
5254
self.assertGreaterEqual(time_of_orbit, start,
5355
f"orbit shift falls short of the ramp for {ramp_ms} ms")
5456

5557
def test_shift_agrees_with_the_timestamp_to_orbit_conversion(self):
5658
"""Closure: the shifted orbit is what main() derives from the shifted timestamp."""
57-
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ITS_RAMPUP_MS)
59+
start, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, ITS_RAMPUP_MS)
5860
# this is the conversion main() uses for the exclude_timestamp() check
5961
derived = FIRST_ORBIT + int((start - SOR) / (anchored.LHCOrbitMUS / 1000.))
60-
self.assertLessEqual(abs(orbit - derived), 1)
62+
# the timestamp has millisecond resolution, which is about 11 orbits
63+
self.assertLessEqual(abs(orbit - derived), anchored.milliseconds_to_orbits(1))
64+
65+
def test_timestamp_is_not_inside_the_ramp_at_a_tf_boundary(self):
66+
"""Regression: a ramp ending just below a TF boundary must not be cut short by the ms truncation."""
67+
ramp_ms = 12928 * anchored.LHCOrbitMUS / 1000. - 0.001
68+
start, _ = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, ramp_ms)
69+
self.assertGreaterEqual(start - SOR, ramp_ms)
6170

6271
def test_split_id_one_clears_the_its_dead_window(self):
6372
"""Regression: the first job of a production must not sample the dead window."""
64-
_, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ITS_RAMPUP_MS)
73+
_, orbit = anchored.shift_anchor_past_ITS_rampup(SOR, FIRST_ORBIT, ORBITS_PER_TF, ITS_RAMPUP_MS)
6574
self.assertGreater(orbit, FIRST_ALIVE_ORBIT)
6675

6776

0 commit comments

Comments
 (0)