Skip to content

Detrending flags#133

Merged
The9Cat merged 9 commits intodevelfrom
detrendingFlags
May 5, 2025
Merged

Detrending flags#133
The9Cat merged 9 commits intodevelfrom
detrendingFlags

Conversation

@michael-petersen
Copy link
Member

This PR promotes totPow and totVar to bools and sets rules for when both are set.

Bigger questions:

  • Is this an issue for backward compatibility (and does it matter)?
  • Is this our long-term detrending strategy?

@The9Cat
Copy link
Member

The9Cat commented May 1, 2025

While sensitive about changing the behavior of an mSSA configuration feature "on the fly", these flags are undocumented, experimental parameters used in a non standard way. Their existence is the toggle unlike other switches which are boolean. So promoting these to boolean with readthedocs and pyEXP dostring description seems sensible, if the consensus is that we keep them. I would rather keep them then remove them. So I'm in favor of @sophialilleengen's proposal. We might consider adding "info" messages confirming the selection when they are used to make the consequences of their choice clear to the user in real time.

I would like to see an implementation of a more general facility detrending options. I'm thinking of a class that provides a detrending and retrending functors for each channel that allow a general time dependent rescaling that a user might define from Python. However, developing this will require additional thought and testing.

@The9Cat The9Cat self-assigned this May 5, 2025
Added spaces for readability
Copy link
Member

@The9Cat The9Cat left a comment

Choose a reason for hiding this comment

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

This all looks good. I will open the desire for a more general detrending functionality as an issue.

@The9Cat The9Cat merged commit c11b5f6 into devel May 5, 2025
8 checks passed
@The9Cat The9Cat deleted the detrendingFlags branch May 5, 2025 14:42
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.

2 participants