Skip to content

CIP-1654: Telescope Model Updates Part 3

Daniel Getz requested to merge cip-1654-telescope-model-updates into master

Generated Docs: https://ska-telescope.gitlab.io/-/ska-telmodel/-/jobs/6397979607/artifacts/docs/build/html/index.html

Thoughts before I depart:

  • This should be merged after !152 (merged) and !156 (CIP-2113 Part 1 & 2). Will make solving merge conflicts easier.
  • Point 18 about xypol needs to be addressed once CIP-2113 Part 1 is in and resolved.
  • Pushed addressing channel_id/offset until ADR-99 is resolved. Will and I both think changing the minimum from 0 to 1 is not a good use of time. A lot of tests will need to be refactored to use 1-indexing instead. Will mentioned possibly bringing it up on the ICD and make it 0 there. Or worst case convert to 0-indexing as early as possible.
  • More range validation tests should be added to ensure the new schema validations are working correctly. In particular, the fields zoom_factor and zoom_window_tuning need tests. I had pushed adding those validation tests in CIP-2249 Part 2 to here since this code review is already adding some range validation in a more modular way. Double check any missing range validation tests to the schemas once the previous parts are merged
  • Redundant range validation needs to be added/removed in this MCS PR: ska-mid-cbf-mcs!333 (closed)
  • I broke down the examples file since most of the examples build on previous iterations. Having to add all new ones with minimal changes every time telescope model changes came in was blowing up the number of lines in this file. I've shrunken it down to around 1000 lines now by consolidating the examples.
  • Add schema range validation for frequency_band_offset_stream. It is +/- 0.5 * Frequency Slice BW (FSBW). The FSBW looks to be 200mhz in MCS. Double check this is correct to add the range validation to the telescope model
  • I think the range validation for the frequency slice/band would be messy to implement in the schemas since the slice value depends on the band value. Might be easier to keep it to the MCS repo.
Edited by Anton Deriabine

Merge request reports