-
Notifications
You must be signed in to change notification settings - Fork 65
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
Use common beam instead of beam averaging #656
Open
e-koch
wants to merge
63
commits into
radio-astro-tools:master
Choose a base branch
from
e-koch:remove_beamaverage
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
1a4ce01
Switch avg -> common beam
e-koch 208a68a
Update beam warning messages for common beam
e-koch 1133a4e
Change test checks to ensure the common beam is being returned
e-koch 9d88227
Add test beam set with similar areas
e-koch 428e185
Looks like we had a duplicated function -> see base_class.py
e-koch 928a51c
Separate beam area checks from avg beam and checks for bad beams; def…
e-koch 7a55cf3
thresholds all default to VRSC.beam_threshold
e-koch f119270
Missed one threshold input
e-koch a821462
Alter test_mask_beam_beams to check for varying setting, but no opera…
e-koch 51dc3ad
More information error message
e-koch 90cc768
Add tests for failure and pass for spectral operation due to beam dif…
e-koch 803468a
Make note for adding in beam area check for DaskVRSC (no checks right…
e-koch 69f734d
Add new fixture to ALL_DATA_FIXTURES
e-koch 8ad3259
Add extra attribute names to handle DaskVRSC w/ a beams check
e-koch cab4b3e
Add VRSC tests for beam checks that fail and pass
e-koch 8cec35c
Remove to-do now handled directly in VRSC
e-koch 0c5e9fb
Clean-up
e-koch e1a50a8
common beam operations require scipy
e-koch fe27f9a
Add back average beams, which now just passes to `set_common_beam` wi…
e-koch 59c52b3
Add setting for a "strict" beam matching mode (i.e., threshold = 0.0 …
e-koch 8b4c076
Add strict beam match mode test
e-koch b010653
Add checks for strict_beam_match in _check_beam_areas. Users will get…
e-koch a000f93
Need to specify axis=0 for spectral operations to trigger the beam ar…
e-koch ac848f3
Clean tests up; use common list of spectral operations
e-koch 179ffbd
Add deprecation test for average_beams
e-koch aa4e61b
Extra self
e-koch 9a3392f
Add a single line unit conversion example for VRSC
e-koch e8a9525
Update smoothing docs for new common beam operations
e-koch d7bc388
Add a bunch of common/bad beam handling text to the docs
e-koch 2cc72f1
Add link to beam handling for common errors
e-koch 24d6458
Just use **kwargs to pass to common beam algorithm
e-koch 162d323
Change default common beam masking to just use goodbeams_mask; descri…
e-koch 3191df5
Check for array mask first. Otherwise the == comparisons give a Futur…
e-koch 2d8315b
Add test for different set_common_beam mask options
e-koch 2dead55
Remove "combeam_kwargs"
e-koch 0d1f6f2
Add unmasked_channels property to cache any(axis=(1,2)) calls on masks
e-koch f9eddca
Add test for `cube.unmasked_channels`
e-koch 5f0bdf3
Need to make an approx comparisons for slightly different rounding in…
e-koch 41d2110
Use unmasked_channels in set_common_beam
e-koch b80e990
Add _mask_include for all Dask cube types
e-koch 5315641
Change default mask for deprecated average_beams
e-koch 01cfc61
Fix wrong numpy testing function
e-koch ce99b95
Fix logic using unmasked_channels and separate da.Array checks
e-koch 7776c03
Fix handling 3D masks
e-koch 8c3b9ba
Minor docs comments from @keflavich
e-koch a9be9e8
Update docs/beam_handling.rst
e-koch bb39274
Update docs/beam_handling.rst
e-koch 145a2bc
Update docs/beam_handling.rst
e-koch 2156ea9
Small fixes from @astrofrog
e-koch 400308c
Update docs/beam_handling.rst
e-koch 3202da0
Allow for computing common beam on init; state set by initial data + …
e-koch 70f2527
Add convenience functions for convolving to a common beam
e-koch a0be2b5
Add `with_bad_beams_masked` and deprecated `mask_out_bad_beams`
e-koch 0aca1a6
PR #87 will raise a BeamError for beams that cannot be deconvolved; a…
e-koch e8a93ac
Try accessing the common beam before any other checks to see if it ex…
e-koch f64f75c
compute_common_beam shouldn't be private
e-koch cf14a69
Change common beam masking changes to test output, not the attribute
e-koch 6f48baf
Update compute_common_beam docs; with_common_beam moving to own PR
e-koch 5206ff4
Fix inline string in docs
e-koch 6e86c8a
casa io tests didn't get removed in the rebase...
e-koch 612326f
Add da.Array as accepted type for masks in low dim objects
e-koch 2044657
Remove wrong warning about triggering a whole cube convolution; that …
e-koch 147d9be
Remove the unneeded beam test against the old averaged beam
e-koch File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why we need to make a stateful change on the cube and set
common_beam
? We could instead do something like:? Having it stateful means it could get out of sync if e.g. some channels are masked and so on. In general we might want to avoid stateful changes to cubes if we can?
With
find_common_beam
above one could even find a common beam with different settings and compare them without having to make a stateful change each time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only issue is that the common beam operation can be slow for cubes with many (>1e3) channels. And baked into the checks for deviations is the need to check for variation from the common beam, where we had the average beam before. That's our only check (currently) for whether to ignore small beam changes for operations along the spectral axis.
Allowing a stateful change could also work if we force the beam masking to not be stateful (which I think it can be now)