Add an RF masterclock facade

[gupichon]

Description

Adds RFHolder.masterclock, returning the RFPlant configured as DEFAULT_RF_PLANT, making the default RF plant explicit. frequency/voltage become backward-compatible aliases for masterclock.frequency/masterclock.voltage instead of duplicating the DEFAULT_RF_PLANT lookup.

Related Issue

• Fixes Feature: Add an RF masterclock facade #374

Features/issues described there are:

• new feature: rf.masterclock was added as a property delegating to the existing get("DEFAULT_RF_PLANT") — no new lookup/error logic needed, since ElementHolder._get() already raises PyAMLException when the name is absent (same convention already used for chromaticity/tune/orbit/etc.).

Changes to existing functionality

• RFHolder.frequency/RFHolder.voltage: reimplemented to delegate to self.masterclock.frequency/self.masterclock.voltage instead of calling self.get("DEFAULT_RF_PLANT") directly, to avoid duplicating the literal name in two places. Behavior and return values are unchanged for existing callers.

Testing

The following tests (compatible with pytest) were added to tests/rf/test_rf.py:

• test_masterclock_returns_the_default_rf_plant
• test_frequency_and_voltage_are_masterclock_aliases
• test_masterclock_raises_when_default_rf_plant_missing
• test_simple_rf_access

Verify that your checklist complies with the project

• New and existing unit tests pass locally (308 passed, 1 pre-existing unrelated failure: tests/tuning_tools/test_tuning_orm.py, a Windows-only NamedTemporaryFile double-open issue)
• Tests were added to prove that all features/changes are effective
• The code is commented where appropriate (NumPy-style docstrings, including the class-level docstring so masterclock is discoverable via help(sr.live.rf))
• Any existing features are not broken

[JeanLucPons]

To me this is a bit confusing, a masterclock does not control the RF voltage.

[gubaidulinvadim]

To me this is a bit confusing, a masterclock does not control the RF voltage.

@gupichon I agree with @JeanLucPons , I wrote the same thing in a linked issue a few days ago.

[JeanLucPons]

Concerning the voltage (or rather the power) of the masterclock, it is tuned to ensure that the signal goes through all cables to be detected by all low level RF elements. Then the RF voltage in various cavity (booster or SR) is regulated by the RF transmitters that include their own low level RF element.

So the masterclock power tuning is rather for RF specialist and not for accelerator physicists. To me it should not be in pyAML.
We can even imagine to transmit the masterclock signal using numeric system such as white rabbit timing system.

[GamelinAl]

To me the masterclock device should only contain the frequency attribute. I agree with @JeanLucPons and @gubaidulinvadim that a voltage atribute for a masterclock does not make sense.

I think we should have a cavity device controlling the voltage and phase (corresponding to the total cavity voltage and cavity phase for the sum of all cavities for a given harmonic). If the synchrotron use only a single harmonic the phase attribute is probably not useful, but it is for multi-harmonic systems.

Both of these different elements could share the RFHolder.

[JeanLucPons]

The actual RF holder provides:

sr.live.rf.frequency.set(...)  # Set main RF freq (the one of the DEFAULT_RF_PLANT, for powered harmonic cav you should pass by its transmitter)
sr.live.rf.voltage.set(...) # Set the total RF voltage of all main RF freq (not including powered harmonic cav)
sr.live.rf.transmitter.get("TRA1").voltage.set(...) # Set the voltage of cavities powered by TRA1
sr.live.rf.transmitter.get("TRA1").phase.set(...) # Set the phase of cavities powered by TRA1

Edit:
sr.live.rf.frequency.set() can also affect powered harmonic cavity, the harmonic order is already a part of the config but (as least for us) it is plan to do that by hardware. So i don't think we will need to tune the frequency of powered harmonic cav.

[gupichon]

Thanks everyone for the feedback. If I understand correctly, I should just revert voltage back to how it was before this PR, and we'll design a separate cavity feature later to cover voltage/phase. Is that right?

[gubaidulinvadim]

@gupichon I think this PR could just be closed. Presently the rf.frequency.get() should already be the masterclock frequency.

[JeanLucPons]

I add that if you have a transmitter that power several cavities and that you need to set phase for each individual cavities this is not possible in pyAML for the moment. For us, we will need this four our transition from klystron (10 cavities) to SSA (1 cavity) but this is rather for RF specialists to ensure good phase between main RF cavities and should not be a part of pyaml.
However, tuning of the phase for powered harmonic cavities may be of interest for physicists and this is already possible using pyaml. For passive harmonic cavity, we may need a dedicated cavity device if physicists need to adjust the tuning angle.
I agree with Vadim concerning this PR.

[gupichon]

Got it, I'm closing it now. Thanks for the review.