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

Improve flagging and noise estimation #695

Merged
merged 22 commits into from
Jan 4, 2024
Merged

Conversation

tskisner
Copy link
Member

We have two primary flag bits for shared and detector data:
"invalid" and "processing". The invalid bit means that the
data does not exist or is wrong / corrupted in some way that
makes it useless for any application. The processing bit
means that the data should not "typically" be used for
science results.

This work attempts to change all built-in simulation and
reduction operators to follow these basic guidelines:

  1. Operators that load data:
  • Boresight pointing marked as invalid if no pointing
    solution exists
  • Detector data marked as invalid if it is missing (e.g.
    dropped packets, etc)
  • In some cases, there may be data that should not
    routinely be used for science, and these samples should
    be flagged with the processing bit.
  1. Operators that simulate telescope observing:
  • Unless the operator is also simulating failed pointing
    solutions or readout problems, no samples should be flagged
    as invalid.
  • Some samples might be flagged with the processing bit if
    those features are typically known by low-level data
    acquisition systems. Examples might be cooler cycles or
    scan turnarounds, although those samples might also be
    flagged by downstream operators.
  1. Operators that simulate detector timestreams:
  • Existing invalid detector samples should have their flags
    passed through and remain invalid
  • Detector samples flagged with the processing bit should have
    this bit passed through.
  • If the simulation requires data inputs (e.g. pointing), and
    that data is flagged as invalid, then the output samples of
    the simulation operator should be flagged as invalid.
  • If the simulation uses inputs flagged with the processing
    bit, it is up to the operator whether to use those samples.
  • If the simulation is unable to generate some samples, then
    those samples should be marked as invalid since they do not
    contain the full expected detector response
  • If the simulation generates poor-quality data for some reason,
    those samples might be marked with the processing flag
  1. Operators that process detector timestreams:
  • Existing invalid detector samples should have their flags
    passed through and remain invalid
  • Detector samples flagged with the processing bit should have
    this bit passed through as well.
  • It is up to the operator whether to use samples flagged with
    the processing bit.
  • If the operator is modifying the detector data, and fills
    some samples with garbage, zeros, etc, then those samples
    should be marked as invalid.
  • If the operator is modifying the detector data, and is unable
    to use / process some data, then those samples should have
    the processing bit set.

This PR also include improvements to flagging in the Offset template and a typo fix in fitting noise models.

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.

We need to work on this PR a little more. There needs to be a distinction between samples that should not be used in template fitting and filtering and samples that are excluded from all processing. Point source flags are a good example of usable but excluded samples. We still want the point sources in the final map. I have flagged completely excluded samples such as turnarounds as "invalid" but now they seem to drift into the processing mask.

src/toast/observation.py Outdated Show resolved Hide resolved
src/toast/observation.py Outdated Show resolved Hide resolved
src/toast/observation.py Outdated Show resolved Hide resolved
src/toast/ops/cadence_map.py Outdated Show resolved Hide resolved
src/toast/ops/crosslinking.py Outdated Show resolved Hide resolved
src/toast/ops/madam.py Outdated Show resolved Hide resolved
src/toast/ops/mapmaker_binning.py Outdated Show resolved Hide resolved
src/toast/ops/mapmaker_templates.py Outdated Show resolved Hide resolved
src/toast/ops/mapmaker_utils/mapmaker_utils.py Outdated Show resolved Hide resolved
src/toast/ops/noise_estimation.py Outdated Show resolved Hide resolved
@tskisner
Copy link
Member Author

Summarizing a discussion from other channels:

Principles

  • Invalid data is a "one way street". Any operator which modifies data and fails to modify some samples should mark those samples as invalid.
  • Default flagging masks across all operators should be self-consistent and "just work" for unit tests and typical usage.
  • Operators that raise flag bits should support taking the mask to raise as a trait, to support custom use cases.
  • Other default flag bits will be defined to support common data selection criteria.

Implementation Plan

  • In the default flag mask definitions, make these changes:
    • det_mask_sso set to “4”
    • turnaround set to “2” and changed to “shared_mask_turnaround”
    • elnod set to “4” and changed to “shared_mask_elnod”
    • Similar rename of other ground shared masks
    • Build composite mask aliases:
      • shared_mask_nonscience = invalid | unstable | irregular
      • det_mask_nonscience = invalid | processing | sso
  • Go through all operators and update defaults to reflect correct mask (usually invalid or nonscience)
  • Go through all operators and ensure that failed data modifications are flagged with invalid.

@tskisner
Copy link
Member Author

Ok @keskitalo , this PR is ready for re-review. There is a small issue when running on GPU with hybrid pipeline data movement, but that will likely just be a typo fix somewhere. Will not merge until those tests pass (which are not tested by CI anyway).

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.

Mostly small observations.

The one big question is about det_flag_mask being repurposed and whether that is general enough.

src/toast/_libtoast/ops_filterbin.cpp Show resolved Hide resolved
src/toast/ops/cadence_map.py Outdated Show resolved Hide resolved
src/toast/ops/demodulation.py Outdated Show resolved Hide resolved
src/toast/ops/demodulation.py Outdated Show resolved Hide resolved
src/toast/ops/filterbin.py Outdated Show resolved Hide resolved
src/toast/ops/statistics.py Show resolved Hide resolved
src/toast/templates/fourier2d.py Outdated Show resolved Hide resolved
src/toast/templates/fourier2d.py Outdated Show resolved Hide resolved
src/toast/templates/gaintemplate.py Outdated Show resolved Hide resolved
src/toast/templates/subharmonic.py Outdated Show resolved Hide resolved
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.

Looks great!

src/toast/templates/offset/offset.py Outdated Show resolved Hide resolved
@tskisner tskisner merged commit a857f1f into hpc4cmb:toast3 Jan 4, 2024
6 checks passed
@tskisner tskisner deleted the nse_robust branch January 4, 2024 14:41
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