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

Add function to filter out ASIC tuning data from mast results #804

Merged
merged 11 commits into from
Oct 19, 2021

Conversation

bhilbert4
Copy link
Collaborator

@bhilbert4 bhilbert4 commented Sep 24, 2021

This PR adds code to the instrument monitors such that any ASIC tuning data returned by the MAST query will be ignored. ASIC tuning data will have suspect pixel values since readout properties will be non-nominal for these data. We don't want these pixel values contaminating measures of dark current, readnoise, etc.

Files are excluded by checking the 'template' keyword in the dictionaries returned by the MAST query.

  • Find out if there are instrument-specific ASIC tuning template names. Currently the code only looks for the value that NIRCam uses.
  • More testing required. Will need to reset the database tables in order to fully test.
  • Add the filtering function to the readnoise, bias, and bad pixel monitors (cosmic ray monitor?)

@bhilbert4 bhilbert4 self-assigned this Sep 24, 2021
@bhilbert4 bhilbert4 linked an issue Sep 24, 2021 that may be closed by this pull request
@bourque
Copy link
Collaborator

bourque commented Sep 27, 2021

Closes #800

@pep8speaks
Copy link

pep8speaks commented Sep 27, 2021

Hello @bhilbert4, Thank you for updating !

Line 697:31: E127 continuation line over-indented for visual indent
Line 697:48: E712 comparison to True should be 'if cond is True:' or 'if cond:'

Line 45:1: E402 module level import not at top of file
Line 46:1: E402 module level import not at top of file
Line 47:1: E402 module level import not at top of file
Line 48:1: E402 module level import not at top of file
Line 49:1: E402 module level import not at top of file
Line 51:1: E402 module level import not at top of file
Line 52:1: E402 module level import not at top of file
Line 53:1: E402 module level import not at top of file
Line 54:1: E402 module level import not at top of file
Line 55:1: E402 module level import not at top of file
Line 56:1: E402 module level import not at top of file
Line 57:1: E402 module level import not at top of file
Line 58:1: E402 module level import not at top of file
Line 59:1: E402 module level import not at top of file
Line 63:1: E303 too many blank lines (3)
Line 346:13: E128 continuation line under-indented for visual indent
Line 346:42: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 381:13: E722 do not use bare 'except'

Line 80:34: E126 continuation line over-indented for hanging indent
Line 384:31: E127 continuation line over-indented for visual indent
Line 384:67: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 756:42: E128 continuation line under-indented for visual indent
Line 767:57: E128 continuation line under-indented for visual indent
Line 784:37: E128 continuation line under-indented for visual indent
Line 785:37: E128 continuation line under-indented for visual indent
Line 790:33: E128 continuation line under-indented for visual indent
Line 791:33: E128 continuation line under-indented for visual indent
Line 792:33: E128 continuation line under-indented for visual indent
Line 793:33: E128 continuation line under-indented for visual indent
Line 794:33: E128 continuation line under-indented for visual indent
Line 795:33: E128 continuation line under-indented for visual indent
Line 796:33: E128 continuation line under-indented for visual indent
Line 973:44: E128 continuation line under-indented for visual indent

Line 47:1: E402 module level import not at top of file
Line 48:1: E402 module level import not at top of file
Line 49:1: E402 module level import not at top of file
Line 50:1: E402 module level import not at top of file
Line 52:1: E402 module level import not at top of file
Line 53:1: E402 module level import not at top of file
Line 54:1: E402 module level import not at top of file
Line 55:1: E402 module level import not at top of file
Line 56:1: E402 module level import not at top of file
Line 57:1: E402 module level import not at top of file
Line 58:1: E402 module level import not at top of file
Line 59:1: E402 module level import not at top of file
Line 60:1: E402 module level import not at top of file
Line 61:1: E402 module level import not at top of file
Line 62:1: E402 module level import not at top of file
Line 63:1: E402 module level import not at top of file
Line 64:1: E402 module level import not at top of file
Line 382:13: E128 continuation line under-indented for visual indent
Line 382:42: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 416:13: E722 do not use bare 'except'
Line 452:13: E722 do not use bare 'except'

Line 340:21: E126 continuation line over-indented for hanging indent

Line 128:22: E124 closing bracket does not match visual indentation

Line 193:13: E722 do not use bare 'except'
Line 532:13: W503 line break before binary operator
Line 539:13: W503 line break before binary operator

Comment last updated at 2021-10-12 19:30:03 UTC

@bhilbert4
Copy link
Collaborator Author

According to the JWST keyword dictionary, there is only one TEMPLATE value that is related to ASIC tuning, so no need to look for instrument specific template values. Of course, if the instrument teams had other templates they wanted to ignore, those templates could be added to the new list in constants.py and they would be skipped.

@bhilbert4
Copy link
Collaborator Author

Testing on the dev server looks good. The dark monitor failed, but due to NIRSpec darks with either bad metadata or in need of better filtering based on array size. This was all after successfully filtering out ASIC tuning data for multiple other apertures. Tests in the bad pixel, bias, and readnoise monitors were also successful.

@bhilbert4
Copy link
Collaborator Author

@bourque I think this is ready for review. I am a little confused about the RTD failure though. From the raw RTD log, it looks like the test is installing Django version 4.0 (which, when I look around, seems not to exist?) See the Django references here: https://readthedocs.org/api/v2/build/14836853.txt

@bhilbert4 bhilbert4 changed the title [WIP]: Add function to filter out ASIC tuning data from mast results Add function to filter out ASIC tuning data from mast results Sep 28, 2021
@bourque
Copy link
Collaborator

bourque commented Sep 28, 2021

Thanks @bhilbert4! I think the RTD failures were fixed in a recent PR; updating the branch appears to have fixed it.

Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

@bhilbert4 Thanks for this fix. The code changes look good! I just had one suggestion on the location of the exclude_asic_tuning function -- I will leave it up to you if you want to make a change. Feel free to merge when you're ready.

@@ -99,6 +99,7 @@
from jwql.database.database_interface import NIRSpecBadPixelQueryHistory, NIRSpecBadPixelStats
from jwql.database.database_interface import FGSBadPixelQueryHistory, FGSBadPixelStats
from jwql.instrument_monitors import pipeline_tools
from jwql.instrument_monitors.common_monitors.dark_monitor import exclude_asic_tuning
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is going to be used in multiple monitors, perhaps it is best to move the function to monitor_utils?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. I remember thinking about this, and then seeing that the bias and readnoise monitor are also importing mast_query_darks from the dark_monitor, and I started thinking I should move both functions to monitor_utils....and then I forgot about it. I'll move both over.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I've moved both functions into monitor_utils.py. There were also two copies of initialize_instrument_monitor(), in monitor_utils.py as well as utils.py, so I removed the latter. This might be worth a quick look just to make sure I didn't screw anything up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Such as a bunch of tests.......Oops.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, tests are all passing again.

@bourque
Copy link
Collaborator

bourque commented Oct 19, 2021

Thanks @bhilbert4 for the updates here! I'm going to merge this now.

@bourque bourque merged commit 87e22a6 into spacetelescope:develop Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude ASIC tuning data from MAST search results
3 participants