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

Cosmic ray rejection parameter in yaml file #1905

Merged
merged 6 commits into from Nov 11, 2022
Merged

Cosmic ray rejection parameter in yaml file #1905

merged 6 commits into from Nov 11, 2022

Conversation

julienguy
Copy link
Contributor

In this PR the preprocessing code looks for the keyword COSMICS_C2 in the calibration yaml file config if the argument is not set.
This is needed to process the new MOSAIC CCD (500 micron-tick device).

Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

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

The actual algorithmic update looks good, including how you handle the default parameter, setting the parameter, or getting it from the config file.

I put two minor stylistic comments inline for the record, but I can make the actual cleanup changes.

@@ -236,21 +236,21 @@ def amplifier_matching(image) :
vband_a = np.median(image[c0-lw:c0,c1-w:c1],axis=1)
vband_b = np.median(image[c0-lw:c0,c1::c1+w],axis=1)
rab = np.sqrt( np.median(vband_a/vband_b) / np.median(vband_b/vband_a) ) # to debias
log.debug("a/b=",rab)
log.debug("a/b={}".format(rab))
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case you are fixing a logging bug, and this statement isn't buried in a loop so performance doesn't matter much, but in general we should get in the habit of formatting log.debug statements with

log.debug("a/b=%f", rab)

The difference is that if the logger knows it won't print the message, it skips evaluating the format string, whereas the way it is currently written the string is evaluated whether or not it will be printed. In the past that has mattered when we accidentally had a formatted debug string inside a nested loop and ended up taking a lot of time just on formatting a debug statement that was normally never printed.

log.error("band image has nan values")
sys.exit(12)

if 1 :
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover debugging if 1? i.e. let's either make this an option that we can genuinely turn on and off, or drop the if 1 and indentation.

@sbailey sbailey merged commit e1c96b5 into daily Nov 11, 2022
@sbailey sbailey deleted the kpno-mosaic branch November 11, 2022 22:53
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