Restrict detectors of weird t2p estimations #1381
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
msilvafe
requested changes
Sep 30, 2025
| The lower and upper limit of acceptable leakage coefficient. | ||
| """ | ||
| if t2p_aman is None: | ||
| t2p_aman = aman.t2p_stats |
There was a problem hiding this comment.
This is not guaranteed to exist. I think you should wrap this in a try-except and raise a useful error.
Comment on lines
+348
to
+359
| redchi2s : bool | ||
| If True, restrict detectors based on redchi2s values. | ||
| error : bool | ||
| If True, restrict detectors based on fit errors of lamQ and lamU. | ||
| lam : bool | ||
| If True, restrict detectors based on the amplitude of lamQ and lamU. | ||
| redchi2s_lims : tuple | ||
| The lower and upper limit of acceptable redchi2s. | ||
| error_lims : tuple | ||
| The lower and upper limit of acceptable errors. | ||
| lam_lims : tuple | ||
| The lower and upper limit of acceptable leakage coefficient. |
There was a problem hiding this comment.
Where are redchi2, error, and lam defined? Refer to appropriate docstring or define here.
Also I think you should just make these be 3 variables and have them default to None and otherwise pass a tuple for the values. In the if redchi2s: line for instance if redchi2s is not it will pass this block, if it's a tuple it will be True and behave as intended.
| else: | ||
| raise ValueError('no leakage coefficients found in axis manager') | ||
|
|
||
| def restrict_dets_from_t2p_fit(aman, t2p_aman=None, redchi2s=True, error=True, lam=False, |
| lam_t2p = np.sqrt(t2p_aman.lamQ**2 + t2p_aman.lamU**2) | ||
| mask_lam = (lam_lims[0] < lam_t2p) & (lam_t2p < lam_lims[1]) | ||
| mask = np.logical_and(mask, mask_lam) | ||
| aman.restrict('dets', aman.dets.vals[mask]) |
There was a problem hiding this comment.
This should only restrict in-place optionally. Add an argument in_place which if true restricts as is done here and if false does not and returns mask.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a function to t2pleakage.py.
The new function
restrict_dets_from_t2p_fitrestricts detectors based on the t2p fit stats.It has three options of t2p fit parameters: redchi2, error, and lam.
Here I show the example TOD of one detector with bad t2p fit. (obs_1712141864_satp1_1111111)
Sometimes residual noise in demodQ/U is quite large due to bad t2p correlation.
This figure shows the histograms of redchi2s in observations with good and bad t2p correlation.
This implies that we can cut the detectors of large t2p residuals based on redchi2s or other fit parameters.
Assuming the valid chi2 value is 0.1<chi2<3.0, I have cut the detectors in 262 observations.
The number of detectors is reduced from about 3300 to 2700; about 600 (18%) detectors are discarded.