Skip to content

Noise fitting frequency limits from hwp freq #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mmccrackan
Copy link
Contributor

@mmccrackan mmccrackan commented Jan 6, 2025

Branch to address #1071. If the config file upper-frequency limits (either the white noise limit or the global upper limit of allowed frequencies) are greater than the hwp frequency (if hwp is present), they will be restricted to 0.95 x hwp frequency (matching demodulation code) and a warning is printed. It will fail if the lower frequency limit ends up being >= upper frequency limit.

Also a small change to the limits on the noise fitting for fknee. Sometimes, zeros can occur and if alpha < 0, this gives NaN warnings.

@mmccrackan mmccrackan requested a review from msilvafe February 20, 2025 14:44
Copy link
Contributor

@msilvafe msilvafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one comment to address and just want to confirm you tested:

  1. when high_f/fwhite[1] are > hwp spin rate you got the expected correction in the values
  2. the appropriate errors are raised when low f > high f
  3. The divide by zero errors are eliminated by the addition of sys.float_info.min in the bounds

logger.warning(f"high frequency cutoff={high_f} > hwp_freq={hwp_freq}. Limiting to hwp_freq.")
high_f = hwp_freq_limit
# make sure low_f < high_f
if low_f >= hwp_freq_limit:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low_f > high_f is a check that should happen regardless of the check on hwp spin rate so I think this check/error raising should happen outside of the hwp_angle field check block.

logger.warning(f"upper white noise freq={fwhite[1]} > hwp_freq={hwp_freq}. Limiting to hwp_freq.")
fwhite[1] = hwp_freq_limit
# make sure fwhite[0] < fwhite[1]
if fwhite[0] >= hwp_freq_limit:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as line 371

@mmccrackan
Copy link
Contributor Author

Looks good, one comment to address and just want to confirm you tested:

  1. when high_f/fwhite[1] are > hwp spin rate you got the expected correction in the values
  2. the appropriate errors are raised when low f > high f
  3. The divide by zero errors are eliminated by the addition of sys.float_info.min in the bounds

Just realized this won't work for the raw signal PSD after testing on the ISO configs. Will fix.

@@ -356,6 +359,18 @@ def calc_wn(aman, pxx=None, freqs=None, low_f=5, high_f=10):
if pxx is None:
pxx = aman.Pxx

# limit upper frequency cutoffs to hwp freq
if 'hwp_angle' in aman._fields:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'hwp_angle' in aman should return true if it exists in its fields. So there is no need to specifically check in _fields

@@ -459,12 +474,28 @@ def fit_noise_model(
subscan=subscan,
**psdargs,
)

# limit upper frequency cutoffs to hwp freq
if 'hwp_angle' in aman._fields:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@msilvafe
Copy link
Contributor

msilvafe commented May 5, 2025

Looks good, one comment to address and just want to confirm you tested:

  1. when high_f/fwhite[1] are > hwp spin rate you got the expected correction in the values
  2. the appropriate errors are raised when low f > high f
  3. The divide by zero errors are eliminated by the addition of sys.float_info.min in the bounds

Just realized this won't work for the raw signal PSD after testing on the ISO configs. Will fix.

We chatted about this but putting it here to not forget. I think the way we want to go forward with this is as follows:

  1. When proc_aman is created in the run function add to the empty axismanager a field called frequency_cutoffs
  2. Initially add a single field called signal which has a float matched to the cutoff of the IIR filter (should be 63 Hz but can compute from the iir filter params)
  3. As additional signals are added and/or lowpass filtered (i.e. demodQ, demodU, dsT) their associated frequency_cutoffs should be added
  4. In the noise calculation code check for the frequency_cutoffs.<signal_name> field for the signal_name you're computing the noise off of. If it exists check that the upper bound is < and if not set it equal to the cutoff if it doesn't exist do nothing.

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.

3 participants