Skip to content

Fix preproc hwp freq logging#1588

Merged
mmccrackan merged 3 commits intomasterfrom
260324_pp_hwp_check
Mar 24, 2026
Merged

Fix preproc hwp freq logging#1588
mmccrackan merged 3 commits intomasterfrom
260324_pp_hwp_check

Conversation

@mmccrackan
Copy link
Copy Markdown
Contributor

Just a quick fix for a logging warning in preprocessing which is misleading. The line will print something like:

Upper freq=2 > hwp_freq=2.1035060065284457. Limiting to hwp_freq

But it is actually comparing the frequency to a frequency cutoff which is less than that value.

Copy link
Copy Markdown
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.

One inline comment but approved.

Comment thread sotodlib/preprocess/processes.py Outdated
if fmax >= frequency_cutoff:
logger.warning(f"Upper freq={fmax} > hwp_freq={hwp_freq}. Limiting to hwp_freq.")
if fmax > frequency_cutoff:
logger.warning(f"Upper freq={fmax} > hwp freq cutoff={np.round(frequency_cutoff, 2)}. Limiting to hwp freq cutoff.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you change from >= to >? Also if the np.round rounding is just for prettifying the logs you can use this notation with f-strings: f"{float:.Nf}" where N is the number of sig-figs you want to show (or you can use .Ng if you want display in scientific notation for large or small floats).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed >= to > since If fmax == frequency_cutoff we don't need to change anything or throw a warning since it is already at the bound.

Updated to use f string.

@mmccrackan mmccrackan merged commit a679242 into master Mar 24, 2026
5 checks passed
@mmccrackan mmccrackan deleted the 260324_pp_hwp_check branch March 24, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants