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

Consistent use of Units in detector data, pixel data, and noise models #593

Merged
merged 5 commits into from
Oct 21, 2022

Conversation

tskisner
Copy link
Member

@tskisner tskisner commented Oct 6, 2022

Work in progress on supporting consistent units between detector data, pixel data, etc.

@tskisner tskisner force-pushed the toast3_units branch 2 times, most recently from 846239c to 11dc02e Compare October 6, 2022 21:52
@tskisner tskisner mentioned this pull request Oct 10, 2022
@tskisner tskisner marked this pull request as ready for review October 11, 2022 03:39
@tskisner
Copy link
Member Author

Ok, this will need rebased after #592 is merged, since there will be conflicts, but this work now includes:

  • Enforce consistent units throughout the detector, pixel, and noise classes.

  • Use units when doing map-domain I/O.

  • Introduce a new Unit trait type for operators and templates, and support this trait type throughout the configuration system.

  • Maintain units in the noise model classes, including for detector inverse variance weights.

  • Check units when assigning to DetectorData objects.

  • Propagate units when duplicating and redistributing Observations.

  • Do not copy internal interval containing all samples when redistributing observations.

  • Scale detector data when doing arithmetic with the Combine operator

  • Replace old collective debug logging with Logger.debug_rank()

Also, this does not solve the slow convergence of the gain template in the case of a "perfect" time domain model. We can work on that in the next PR.

* Enforce consistent units throughout the detector, pixel, and noise classes.

* Use units when doing map-domain I/O.

* Introduce a new Unit trait type for operators and templates, and support
  this trait type throughout the configuration system.

* Maintain units in the noise model classes, including for detector inverse
  variance weights.

* Check units when assigning to DetectorData objects.

* Propagate units when duplicating and redistributing Observations.

* Do not copy internal interval containing all samples when redistributing
  observations.

* Scale detector data when doing arithmetic with the Combine operator

* Replace old collective debug logging with `Logger.debug_rank()`

* Run format_source.sh
Copy link
Member

@keskitalo keskitalo left a comment

Choose a reason for hiding this comment

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

This PR makes TOAST significantly more robust against confusion over physical units.

src/toast/ops/crosslinking.py Outdated Show resolved Hide resolved
src/toast/ops/mapmaker_binning.py Show resolved Hide resolved
src/toast/ops/polyfilter.py Show resolved Hide resolved
src/toast/ops/scan_healpix.py Outdated Show resolved Hide resolved
src/toast/ops/scan_map.py Outdated Show resolved Hide resolved
src/toast/ops/scan_wcs.py Outdated Show resolved Hide resolved
src/toast/templates/gaintemplate.py Show resolved Hide resolved
src/toast/tests/_helpers.py Show resolved Hide resolved
src/toast/tests/accelerator.py Show resolved Hide resolved
Copy link
Contributor

@giuspugl giuspugl left a comment

Choose a reason for hiding this comment

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

thanks a lot to put together this PR. It is a huge effort and would address several open issues e.g. #585
I am happy with the changes! and I approve'em all.

* Fix support for container parsing from CLI.

* Expand unit test coverage.

* Change PixelsWCS bounds to a flat container, not nested.
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.

None yet

3 participants