Skip to content

Conversation

@rikvl
Copy link
Contributor

@rikvl rikvl commented Nov 24, 2025

This PR is to

  • get the frequency masks here in ch_util.rfi and the one in wtl.broker-utils synchronized again and
  • add the new RFI band around 650 MHz to the mask.

I suggest to review the changes here and afterwards I'll make a PR in wtl.broker-utils to add the same changes.

In this PR I did the following:

  • Reorder the entries in the list so they are sorted in frequency
  • Duplicated comment lines so each entry has its own comment
  • Add missing masked bands found in wtl.broker-utils
  • Use more conservative end dates found in wtl.broker-utils
  • Add new RFI band at 642.00 to 652.00

@rikvl
Copy link
Contributor Author

rikvl commented Nov 24, 2025

Questions:

  1. If a small part of a frequency channel is included in this mask, will the entire channel be excluded?
  2. There's a region around 460-470 MHz with conflicting mask in the two repos. Do you have ideas for how to resolve the differences?

@rikvl rikvl requested review from ljgray and ssiegelx November 24, 2025 17:09
@rikvl rikvl changed the title Rvl/bad freqs fix(rfi): revisit frequency mask Nov 24, 2025
@ketiltrout
Copy link
Member

  1. If a small part of a frequency channel is included in this mask, will the entire channel be excluded?

Yes.

fmask = (freq_end > fs) & (freq_start < fe)

@ljgray
Copy link
Contributor

ljgray commented Nov 24, 2025

  1. There's a region around 460-470 MHz with conflicting mask in the two repos. Do you have ideas for how to resolve the differences?
    Personally, I would tend towards using the mask from wtl in case of conflicts.

@ljgray
Copy link
Contributor

ljgray commented Nov 24, 2025

I think that this is outside of the scope of this PR, but we should also try to check whether any of these bands have improved since they were masked. It might be the case that some narrowband stuff was only temporary and could be recovered.

@rikvl
Copy link
Contributor Author

rikvl commented Nov 24, 2025

Thanks for the feedback!

Personally, I would tend towards using the mask from wtl in case of conflicts.

Okay, I followed this advice. Note that this means the ch_util-only band 464.00 - 470.32 MHz starting at first light is removed, replaced with the two wtl-only bands 464.45 - 465.24 MHz and 468.75 - 469.54 MHz starting 2023-11-21. If you think that's a bad idea, please let me know

@rikvl
Copy link
Contributor Author

rikvl commented Nov 24, 2025

I think that this is outside of the scope of this PR, but we should also try to check whether any of these bands have improved since they were masked. It might be the case that some narrowband stuff was only temporary and could be recovered.

I agree. Can you remind me in what data you would investigate this?

@ljgray
Copy link
Contributor

ljgray commented Nov 24, 2025

I think that this is outside of the scope of this PR, but we should also try to check whether any of these bands have improved since they were masked. It might be the case that some narrowband stuff was only temporary and could be recovered.

I agree. Can you remind me in what data you would investigate this?

I would check the raw ADC data, and whatever metric Seth used to derive the wtl mask since we know that the raw ADC doesn't capture this new RFI band at the very least.

@rikvl
Copy link
Contributor Author

rikvl commented Dec 2, 2025

Ran into this. Just leaving a note here so I remember.

It looks like this Theremin graph is yet another place with a hardcoded frequency mask. This can presumably be replaced with an import from ch_util:
https://github.com/chime-experiment/theremin/blob/2634b4b9d193f6e2e6a8ab57854c3efd9695d3d8/theremin/graphs/senswaterfall.py#L40-L58

@ljgray
Copy link
Contributor

ljgray commented Dec 4, 2025

Ran into this. Just leaving a note here so I remember.

It looks like this Theremin graph is yet another place with a hardcoded frequency mask. This can presumably be replaced with an import from ch_util: https://github.com/chime-experiment/theremin/blob/2634b4b9d193f6e2e6a8ab57854c3efd9695d3d8/theremin/graphs/senswaterfall.py#L40-L58

Oh man, that's old. We should definitely update it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants