Skip to content
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

Assorted bug fixes #275

Merged
merged 5 commits into from
Jun 12, 2024
Merged

Assorted bug fixes #275

merged 5 commits into from
Jun 12, 2024

Conversation

ljgray
Copy link
Contributor

@ljgray ljgray commented Jun 1, 2024

Some miscellaneous bug fixes:

  • Use np.ndarray instead of mpiarray.MPIArray in MaskBaselines. This was causing an error in python 3.11+ because ufunc where is not defined for distributed arrays
  • Change the minimization method used by minimize_scalar in MaskFreq. The previous method golden would fail with newer versions of scipy because it could not find a valid bracket. I believe that this is because the function being minimized is roughly monotonically decreasing, so the criteria func(xb) < func(xa) and func(xb) < func(xc) could not be met. (See: https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize_scalar.html). I also tried using a standard scipyoptimize.minimize here. The only method which would converge properly was COBYLA, but it sometimes produced a slightly different mask even with appx. the same minimum at a 1e-4 tolerance.
  • Added a proper nsigma initial threshold for the sumthreshold algorithm in RFIStokesI and a variance estimate. Previously, the default threshold of the 95th percentile was being used, which causes problems when previous masks have been applied. The variance is calculated in a slightly wonky way, but it works well to apply sumthreshold over daytime data. A variance boost of 1e4 is added during solar transit, and max_m is increased for better masking.
  • Fixes a misnamed variable in sumthreshold

With these changes, we can run the pipeline with the newest version of python available on Cedar (3.11.5), which has a built module chime/python/2024.04

@ljgray ljgray changed the title Fixes for python 3.11+ Assorted bug fixes Jun 3, 2024
@ljgray ljgray marked this pull request as ready for review June 4, 2024 00:37
Copy link
Contributor

@jmaceachern jmaceachern left a comment

Choose a reason for hiding this comment

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

Looks great. I think after double checking Seth's point about units, this looks good to merge.

draco/analysis/flagging.py Show resolved Hide resolved
draco/analysis/flagging.py Outdated Show resolved Hide resolved
draco/analysis/flagging.py Outdated Show resolved Hide resolved
The minimization routine used to find an optimal factorizable mask
used method 'golden', which would fail to find a proper bracket
with scipy > 3.8 (used in python 3.10). With more up-to-date versions
of scipy, we need to use a simpler 'bounded' minimization
@ljgray ljgray merged commit ff6241d into master Jun 12, 2024
4 checks passed
@ljgray ljgray deleted the ljg/fix-ufunc branch June 12, 2024 23:03
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