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

Review for DM-5161: Support a full background model when detecting cosmic rays #29

Merged
merged 4 commits into from Feb 29, 2016

Conversation

jdswinbank
Copy link
Contributor

No description provided.

exposure0 = exposure # initial value of exposure
binSize = self.config.cosmicray.background.binSize
nx, ny = exposure.getWidth()/binSize, exposure.getHeight()/binSize
if nx*ny <= 1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the logic here, but: what happens if we call estimateBackground even when nx*ny is less than 1?

  • If it uses the median, can we just call it here? It simplifies the logic since we don't need to handle this case specially.
  • If it doesn't just use the median, what's the justification for using the same technique here as in estimateBackground for small bins, and a different technique for large bins?

Copy link
Contributor

Choose a reason for hiding this comment

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

The way I see it, the current logic is much simpler for the nx*ny<=1 case. This is because the estimateBackground function returns an exposure and a lsst.afw.math.mathLib.Background object (computed based on the config values, for which the default is indeed set to MEDIAN here). The variable bg fed into findCosmicRays is a double, so I would still need to compute that with afwMath.makeStatistics(bg.getImageF(), afwMath.MEDIAN).getValue() after having run bg, exposure = measAlg.estimateBackground(exposure, self.config.cosmicray.background, subtract=False) and note that I have set subtract=False in the above as we do not actually want a background subtracted copy of the image here (although I suppose we could do that as in the nx*ny>1 case, but that seems like an extra operation that is not required for the majority of cases).

I can confirm that the MEDIAN sky level is the same for the case I gave above as it is in the current implementation.

In all, I am inclined to leave the current logic.

If there are strong gradients (e.g. M31's nucleus) we need to do more than
treat the background as a constant.  However, this requires making a copy
of the data so the background-is-a-constant model is preserved as a special
case

Conflicts:
	python/lsst/pipe/tasks/repair.py
When the background is subtracted with finer binsize, new exposure
will be created and cosmic rays will be detected on that exposure.
But the image of that exposure was not properly returned back.

Conflicts:
	python/lsst/pipe/tasks/repair.py
@laurenam laurenam merged commit 0e8e373 into master Feb 29, 2016
@timj timj deleted the u/lauren/DM-5161 branch February 18, 2021 15:51
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

4 participants