Skip to content

Fix warning about log-linear frequency spacing#1959

Open
rossjjennings wants to merge 2 commits intonanograv:masterfrom
rossjjennings:fix-log-freq-warning
Open

Fix warning about log-linear frequency spacing#1959
rossjjennings wants to merge 2 commits intonanograv:masterfrom
rossjjennings:fix-log-freq-warning

Conversation

@rossjjennings
Copy link
Member

In PR #1877, @vhaasteren introduced a new log-linear frequency spacing mode. Unfortunately, some of the associated code gives rise to a bunch of spurious warnings of the form

WARNING (pint.models.noise_model:1319): Log-spaced parameters are ignored because logmode, nlog,
and f_min ALL neeed to be setUse: logmode > 0 and nlog > 0 and f_min > 0.

when not using the new feature (i.e., using purely linearly-spaced frequencies, corresponding to logmode=0). These show up because pint.models.noise_model.get_rednoise_freqs() is passed a positive value of f_min by default, regardless of logmode. Because, with logmode=0 and nlog=None, any value of f_min other than 0 or None triggers the warning, it shows up even under the default settings.

This PR changes the conditions for triggering the warning so that it doesn't show up in the case where f_min is nonzero but logmode=0 and nlog=None, which is the default when not using log-spaced frequencies. This is safe because f_min is ignored in this case anyway. I've also edited the warning message to hopefully make it more helpful if it does appear.

@vhaasteren vhaasteren self-requested a review January 20, 2026 09:54
Copy link
Member

@vhaasteren vhaasteren 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 to me, this is ready for merge

@vhaasteren
Copy link
Member

Thanks for noticing this @rossjjennings . Those warnings were not covered by the unit tests, and strictly speaking nothing breaks, so that slipped through. Ready to merge

@dlakaplan
Copy link
Contributor

I can merge this whenever you are happy.

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

Comments