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-40186: Update dynamic detection task defaults and logging #349

Merged
merged 4 commits into from Aug 16, 2023

Conversation

laurenam
Copy link
Contributor

No description provided.

@laurenam laurenam requested a review from TallJimbo July 31, 2023 00:15
@@ -123,6 +123,12 @@ def calculateThreshold(self, exposure, seed, sigma=None):
sigma : `float`, optional
Gaussian sigma of smoothing kernel; if not provided,
will be deduced from the exposure's PSF.
minFractionSourcesFactor : `float`
Change the fraction of required sky sources from that set in
``self.config.minFractionSources`` by this factor.
Copy link
Member

Choose a reason for hiding this comment

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

Might want to document here that this is intended for use in the background-tweak pass (even though it's just one method for both passes), since using this instead of just modifying minFractionSources in the first threshold-setting pass would just be confusing (and I was at first quite confused to find the option here, given the method's name).

f"{nGoodPix} ({100*nGoodPix/nPix:.1f}%) in this patch, so there should be "
"sufficient area to locate suitable sky sources. Something is amiss. "
"(Note that the fraction of \"good\" pixels that have the DETECTED or "
f"DETECTED_NEGATIVE mask bit set is {100*nDetectedPix/nGoodPix:.1f}%).")
Copy link
Member

Choose a reason for hiding this comment

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

Propose rewording this:

However, {nGoodPix}/{nPix} pixels are not marked NO_DATA or BAD, so there should be sufficient area to locate suitable sky sources. Note that {nDetectedPix} were marked as DETECTED or DETECTED_NEGATIVE.

It was the "However there are Number (%s)" beginning in particular that didn't read well to me, but I also have a slight preference for putting the original numbers into the logs directly, even if that makes the reader compute some percentages themself.

As the number of input visits increases, the number of pixels with the
INTRP mask bit set increases, which eventually leads to a very large
fraction pixels with this mask bit set.  Thus, if INTRP is included in
the list of masks to avoid when laying down sky objects, very deep
coadds will end up with little-to-no available locations for sky
objects, leading to a failure in the dynamic detection.  Thus, the
safest thing to do is leave it out of the masks to avoid list.
In the dynamic detection task, compute and log the number of "good"
pixels in the exposure, where "good" is defined simply as not having
either the NO_DATA or BAD mask bit set.  If this fraction is greater
than 20% of the total number of pixels, yet we still fail to locate
enough spots to lay down sky objects, raise an RuntimeError exception.
The message includes the fraction of the "good" pixels marked either
DETECTED or DETECTED_NEGATIVE as this is a likely reason for this
situation.
@laurenam laurenam merged commit 6f40f42 into main Aug 16, 2023
2 checks passed
@laurenam laurenam deleted the tickets/DM-40186 branch August 16, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants