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

DM-41952: Add flux sampling of full covariance model to preKernel #225

Merged
merged 1 commit into from Jan 26, 2024

Conversation

Alex-Broughton
Copy link
Contributor

No description provided.

@Alex-Broughton Alex-Broughton force-pushed the tickets/DM-41952 branch 2 times, most recently from 57631de to 5754573 Compare January 17, 2024 18:58
Copy link
Contributor

@plazas plazas left a comment

Choose a reason for hiding this comment

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

Thanks for this! I left a few comments and requested a few changes.

python/lsst/cp/pipe/makeBrighterFatterKernel.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/makeBrighterFatterKernel.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/makeBrighterFatterKernel.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/makeBrighterFatterKernel.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/makeBrighterFatterKernel.py Outdated Show resolved Hide resolved
@Alex-Broughton
Copy link
Contributor Author

I ran the test case for me, but do you mind giving it a look over?

BTW: here's the pipeline I used to test this ticket branch:

description: cp_pipe brighter-fatter kernel calibration construction.
tasks:
  bfkSolve1:
    class: lsst.cp.pipe.BrighterFatterKernelSolveTask
    config:
      connections.inputPtc: ptc
      connections.outputBFK: bfk_10000adu
      covModelFluxSample: {'ALL_AMPS': 10000}
      useCovModelSample: True
      correlationQuadraticFit: False
      correlationModelRadius: 3
      correlationModelSlope: -1.71
      forceZeroSum: True
  bfkSolve2:
    class: lsst.cp.pipe.BrighterFatterKernelSolveTask
    config:
      connections.inputPtc: ptc
      connections.outputBFK: bfk_20000adu
      covModelFluxSample: {'ALL_AMPS': 20000}
      useCovModelSample: True
      correlationQuadraticFit: False
      correlationModelRadius: 4
      correlationModelSlope: -1.71
      forceZeroSum: True
  bfkSolve3:
    class: lsst.cp.pipe.BrighterFatterKernelSolveTask
    config:
      connections.inputPtc: ptc
      connections.outputBFK: bfk_30000adu
      covModelFluxSample: {'ALL_AMPS': 30000}
      useCovModelSample: True
      correlationQuadraticFit: False
      correlationModelRadius: 4
      correlationModelSlope: -1.71
      forceZeroSum: True
  bfkSolve4:
    class: lsst.cp.pipe.BrighterFatterKernelSolveTask
    config:
      connections.inputPtc: ptc
      connections.outputBFK: bfk_40000adu
      covModelFluxSample: {'ALL_AMPS': 40000}
      useCovModelSample: True
      correlationQuadraticFit: False
      correlationModelRadius: 4
      correlationModelSlope: -1.71
      forceZeroSum: True
  bfkSolve5:
    class: lsst.cp.pipe.BrighterFatterKernelSolveTask
    config:
      connections.inputPtc: ptc
      connections.outputBFK: bfk_50000adu
      covModelFluxSample: {'ALL_AMPS': 50000}
      useCovModelSample: True
      correlationQuadraticFit: False
      correlationModelRadius: 4
      correlationModelSlope: -1.71
      forceZeroSum: True

bps-bfk.yaml:

pipelineYaml: "~/pipelines/cpBfkSolve_covSample.yaml"

project: dev
campaign: quick
computeSite: s3df

# Make sure these values correspond to ones in the bin/run_demo.sh's
# pipetask command line.
payload: # defaults to u/abrought/...
  payloadName: "dev-tests/bfk.1.21.2024.v12"
  butlerConfig: "/repo/ir2"
  inCollection: "u/abrought/run6/ptc.2024.01.09.run13540,u/abrought/tests/cti.2024.01.09.version1d,u/lsstccs/defects_13531_w_2023_41/20231111T010016Z,u/lsstccs/flat_13531_w_2023_41/20231111T005006Z,u/lsstccs/dark_13531_w_2023_41/20231111T004259Z,u/lsstccs/bias_13531_w_2023_41/20231111T003418Z,LSSTCam/raw/all,LSSTCam/calib,LSSTCam/photodiode"
  dataQuery: "instrument='LSSTCam' AND exposure IN (2023111301238) AND detector IN (23)"

Copy link
Contributor

@plazas plazas left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments! I'm happy if Jenkins is happy

@Alex-Broughton Alex-Broughton merged commit 67e2e30 into main Jan 26, 2024
2 checks passed
@Alex-Broughton Alex-Broughton deleted the tickets/DM-41952 branch January 26, 2024 23:48
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

2 participants