Data Quality plugin for Imviz #2767

bmorris3 · 2024-03-22T20:34:43Z

This PR implements a Data Quality plugin, with a focus on Imviz. This is part of a larger effort which will support other configs too.

The Data Quality plugin allows a user to load science data and an associated DQ array (using #2761). Imviz will recognize DQ extensions at load time, identify the correct flag mapping (either JWST or Roman, for now; see #2765).

The plugin will assign colors to the flags, and show the DQ array in the viewer. Pixels without flags have no additional color overlay (via glue-viz/glue#2468). The data quality for a pixel is shown in the coordinate info in the top toolbar by appending (DQ: <int>) on pixels with flags (screenshot below).

There's a multi-select dropdown labeled "Filter by bits" where a user can select flags to visualize by checking any of the decomposed bits that each flag contains. The visibility and color of each flag can be set manually.

There's a relative opacity slider for the DQ layer, which sets the DQ layer opacity relative to the science layer. There's a callback on the science layer opacity, so they stay in sync.

Demo video

I took the unusual step to record a narrated, five-minute screen recording with a DQ demo, to help you see the features. The video is too big for GitHub, so you can view it on Box. One feature that is not demonstrated in this video is color selection – if you click the color of the DQ flag in the plugin, you can select any color you like from a full color palette.

Example notebook

Try out the concept notebook included in this PR.

Other notes

I used the no-changelog-entry-needed label because this should produce no user facing changes once we do the last item on the to-do list below.
I could use advice from a wizard (@kecnry 🧙🏻?) to improve the vertical spacing of the vue elements.

Tickets addressed

Three tickets are completed with this PR, and one is partly addressed.

✅ JDAT-4350: implement DQ plugin UI features
✅ JDAT-4347: DQ mouseover info
✅ JDAT-4352: Provide controls for DQ visibility options
🟨 JDAT-4351: Investigate DQ bit logic from the UI
- It is possible to do one specific type of bit logic with the plugin implemented in this PR. You can use the "filter by bits" dropdown to find any DQ flags containing bits, e.g., "show flags that when decomposed contain 2, 3, or 5."
❌ JDAT-4390: DQ plugin tests
- this PR doesn't introduce tests specific to DQ. Testing will be added later on for this ticket.

To do list

minpin latest glue release whenever the next release occurs (containing Preserve alpha while compositing with cmap.set_bad glue-viz/glue#2468)
before merge into main, use Context-aware plugin relevancy #2711 (thanks @kecnry!) to exclude plugin from user sessions until we're ready to make it public, by uncommenting this line

Change log entry

Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
Do the proposed changes follow the STScI Style Guides?
Are tests added/updated as required? If so, do they follow the STScI Style Guides?
Are docs added/updated as required? If so, do they follow the STScI Style Guides?
Did the CI pass? If not, are the failures related?
Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
After merge, any internal documentations need updating (e.g., JIRA, Innerspace)? 🐱

codecov · 2024-03-22T20:52:20Z

Codecov Report

Attention: Patch coverage is 88.96321% with 33 lines in your changes are missing coverage. Please review.

Project coverage is 88.82%. Comparing base (f1ba4a6) to head (a6de416).
Report is 34 commits behind head on main.

❗ Current head a6de416 differs from pull request most recent head a7e748b. Consider uploading reports for the commit a7e748b to get more accurate results

Files	Patch %	Lines
...z/configs/default/plugins/data_quality/dq_utils.py	80.35%	11 Missing ⚠️
jdaviz/configs/imviz/plugins/parsers.py	78.57%	9 Missing ⚠️
...nfigs/default/plugins/data_quality/data_quality.py	94.93%	8 Missing ⚠️
jdaviz/app.py	20.00%	4 Missing ⚠️
jdaviz/core/template_mixin.py	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2767      +/-   ##
==========================================
+ Coverage   88.72%   88.82%   +0.10%     
==========================================
  Files         110      111       +1     
  Lines       16353    16879     +526     
==========================================
+ Hits        14509    14993     +484     
- Misses       1844     1886      +42

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kecnry · 2024-03-25T14:17:15Z

jdaviz/configs/default/plugins/data_quality/data_quality.py

+    # TODO: uncomment this line before merging into main:
+    # irrelevant_msg = Unicode("Data Quality plugin is in development.").tag(sync=True)


you can always uncomment this and just reset it to empty in your own testing notebook and any tests 🤷‍♂️

Of course! I just wanted devs to be able to play with this by checking out the PR, without having to change source code to use it.

is the intention still to include this or is it mature enough that we can remove it entirely and have it enabled in main on merge (and whenever 3.10 is released)?

(we could also have this be considered irrelevant if there are no data quality layers available... 🤔 )

I’m in favor of keeping it out until we decide it’s more stable. It’s likely some things will change when we add support for other configs. Hopefully they aren’t major things, but just in case, let’s defer.

bmorris3 · 2024-03-27T19:45:34Z

I got some early ~"user feedback" by giving a demo today to @tddesjardins, who will be a Roman-focused DQ plugin user. Tyler likes where things are headed. The outstanding tasks in our epic that he specifically touched on in our conversation included:

Show DQ bit (with or without its list of decomposed values) in mouseover info, ideally with the flux from the science layer
It's OK to limit bit selection from the UI to only (a) toggle specific bits, and (b) show/hide all bits. Roman data will most often be accessed through Imviz on the science platform, where the user must be using a Jupyter notebook, and could use the API to load their highly customized masks.

Feature ideas from Tyler that we don't yet have ticketed:

It would be really really useful to pass on DQ information to other plugins. For example, if I load science data with a DQ array, any pixel with any DQ value other than "GOOD" should be masked in aperture photometry. That way I can "trust the photometry results more" (at least with understood caveats).

Note from Brett on the above request: the DQ-to-boolean-mask part of this request is trivial to implement. The slightly less trivial effort is adding the infrastructure to the aperture photometry plugin to check science data for an associated DQ array loaded in the viewer, and passing that along to photutils. But if accomplish that effort, it would be almost no extra effort to pass along the ERR array to photutils too, which would also give us uncertainties from photutils-derived outputs.

Further note on the above note: DQ arrays only exist for L2 products, but ERR arrays exist for L2+. So while it might seem like a narrow use-case (only L2) to implement this for DQ, the ERR use-case applies much more broadly (L2 and beyond).

javerbukh · 2024-03-27T19:52:01Z

@bmorris3 Would PR #2718 be helpful in accessing the DQ array associated with the science data?

camipacifici · 2024-03-27T21:01:01Z

Thank you very much for the feedback @tddesjardins!!
Propagated uncertainties on aperture photometry are on the wish list. Not sure when we will implement though.
I agree that passing the DQ to other plugins would be extremely useful, but as it is now, the plugins are sort of designed to work on level 3 products, which usually do not carry the DQ (at least for JWST). In any case, I will add this as well to the wishlist and see what we can do!

bmorris3 · 2024-03-27T23:55:43Z

@javerbukh Thanks for mentioning that PR – I have a ticket for doing what you did there for all configs, so I'm glad to see you already got Cubeviz 🙌🏻.

bmorris3 · 2024-03-28T15:04:29Z

I got more feedback from giving a demo to @rgcosentino:

important, simplifying UI feature request: add a multiselect dropdown for selecting specific bits to show in the decoded list of flags in the plugin. use AND logic to combine multiple selected flags, i.e.: “when I click to select flags 2, 10, and 18, only show flags that contain jumps AND nonlinearity AND etc…”
can we summarize the fraction/number of pixels with a given flag in the UI? can we list the multiselect dropdown in descending order of the most frequently occurring flags?
consider: should we keep (or giv an option to keep) DQ visible while blinking in Imviz? if you have a source in multiple exposures, you might want to check which exposures are not affected by certain flags
future: this would be handy when visualizing ramps! 👀

camipacifici · 2024-03-28T15:14:16Z

Thank you @rgcosentino!!
About the blinking keeping the DQ visible, @bmorris3 do you have a sense how hard/easy that is?

bmorris3 · 2024-03-28T15:20:06Z

@camipacifici It's easy to implement, but may not be very performant. I'll look into it 👌🏻

bmorris3 · 2024-03-28T20:32:00Z

Today I added @rgcosentino's UI feature request, which solves a couple of the UI challenges in one go:

dq-bit-filter.mov

bmorris3 · 2024-04-08T20:50:28Z

jdaviz/utils.py

+def layer_is_not_dq(data):
+    return not data.label.endswith('[DQ]')


The surest way to identify a DQ layer is to rely on the data label if we're always producing it consistently. I think we can rely on this for now as written, but we could instead, e.g., add a private metadata flag.

In #2718 there is a app._jdaviz_helper._loaded_mask_cube variable that we can use instead of this. That way, if the DQ data is renamed, the _loaded_mask_cube should be able to track that (it does not currently do that in that PR but it would be a useful addition). We've been moving away from using data labels since we are planning (at some point in the future) to enable label renaming. I'm also worried about the scenario where a user manually loads the DQ layer with a custom name, which would mean it is not caught by this line.

@javerbukh - but we intentionally want to be able to support multiple DQ layers. I agree that the eventual solution is probably to move all of this "what type of data is this entry considered" into private metadata that can be populated by the parser.

bmorris3 · 2024-04-08T20:52:48Z

jdaviz/configs/imviz/plugins/coords_info/coords_info.py

+            if associated_dq_layer is not None:
+                if np.isnan(dq_value):
+                    dq_text = ''
+                else:
+                    dq_text = f' (DQ: {int(dq_value):d})'
+            else:
+                dq_text = ''
+            self.row1b_text = f'{value:+10.5e} {unit}{dq_text}'


This is the simplest and fastest possible DQ injection into coords-info that I could do. We could also decompose the bit flag, but that is (1) hard to fit in the horizontal space, and (2) a tiny bit more expensive.

jdaviz/configs/default/plugins/data_quality/data_quality.py

javerbukh · 2024-04-11T18:14:09Z

The DQ plugin so far is a great addition to Imviz, nice work! I left a couple comments in the code but I also want to mention the DQ relative opacity slider. I was confused at first why only the DQ array was loaded, until I moved that slider and saw the data underneath. Is there a reason we cannot use the opacity sliders in plot options to handle this functionality?

I was also nervous to see the amount of lines added and no test coverage for them. I understand that adding the bulk of test coverage will be a follow-up effort, but can you add some simple tests just to show the intended workflow with this plugin? Let me know your thoughts, I'm happy to re-review again soon because it would be great to get this merged!

bmorris3 · 2024-04-15T14:07:25Z

@javerbukh:

I was confused at first why only the DQ array was loaded, until I moved that slider and saw the data underneath.

By default the DQ opacity is 0.9 * (science opacity). Do you think it would be less confusing if we brought it down to 0.5 or so?

Is there a reason we cannot use the opacity sliders in plot options to handle this functionality?

I agree that DQ opacity is a "plot option" in the literal sense, but we need a different strategy for the DQ opacity slider than for the other layers. We wouldn't allow the user to change any but one of the options (opacity) on DQ layers – not the colormap, bias, contrast, stretch, etc. And DQ opacity is defined relative to the science layer opacity, which would mean its opacity slider behaves differently from the others.

None of the above is to say that we couldn't put DQ plot options in Plot Options, but that we'd have to find a way to make these subtleties less confusing.

kecnry

This is really coming along nicely (and quickly)! Just a few comments from a first pass - would you mind exposing whatever you intend to expose and think is fairly stable through the user API so that its obvious what is expected to be tested directly vs internally?

jdaviz/configs/default/plugins/data_quality/data_quality.py

bmorris3 · 2024-04-15T20:14:57Z

Thank you @kecnry, all your points are now addressed.

kecnry · 2024-04-16T13:07:24Z

jdaviz/configs/default/plugins/data_quality/data_quality.py

+
+            dq_layer.state.alpha = self.dq_layer_opacity
+
+    def update_visibility(self, index):


is index (here and in update_color) convenient for the user? This isn't a big deal since we're not exposing this in the user API yet, but definitely something to think about as we play with this in workflows, etc.

Good point. Right now, the index corresponds to an entry in self.decoded_flags, which is a list of dictionaries. We could instead choose to make decoded_flags a dictionary, where the key is the flag. I made it a list because I had better luck iterating over a list in the vue file than a dict.

Right, but even if it internally is a dictionary, maybe we should support passing the key and just internally iterating to look up the entry to change?

kecnry · 2024-04-16T13:09:17Z

jdaviz/configs/default/plugins/data_quality/data_quality.py

+    # TODO: uncomment this line before merging into main:
+    # irrelevant_msg = Unicode("Data Quality plugin is in development.").tag(sync=True)


(we could also have this be considered irrelevant if there are no data quality layers available... 🤔 )

kecnry · 2024-04-16T13:12:12Z

jdaviz/configs/default/plugins/data_quality/data_quality.py

+    @property
+    def user_api(self):
+        return PluginUserApi(
+            self,
+            expose=(
+                'science_layer', 'dq_layer',
+                'decoded_flags', 'flags_filter'
+            )
+        )


for now, I think we also at least want viewer (and possibly flag_map - but that would require using a PluginSelectComponent for that instead of handling the items manually here). How about dq_layer_opacity?

I do think it makes sense to hold back on the color/visibility methods if we're not sure of their API yet.... but eventually I'm guessing we'll want those exposed as well.

Done in c2cd379.

javerbukh

I noticed a strange bug where if I filter by bit (10 in the concept notebook in this PR), then change the color of 1311747 for example, the pixels are no longer colored in at all. The color comes back if I press "clear filter".

…pt nb

javerbukh

Works great! Once CI is working I'll approve.

javerbukh

LGTM!

bmorris3 · 2024-04-17T14:39:30Z

Last remaining failure exists on main and is expected (for now). Thanks all!

…scope#2767

github-actions bot added imviz plugin Label for plugins common to multiple configurations labels Mar 22, 2024

bmorris3 added this to the 3.10 milestone Mar 25, 2024

kecnry reviewed Mar 25, 2024

View reviewed changes

bmorris3 force-pushed the dq-decode-2 branch from b5eb8e0 to e960128 Compare April 8, 2024 20:47

bmorris3 commented Apr 8, 2024

View reviewed changes

bmorris3 added the no-changelog-entry-needed changelog bot directive label Apr 8, 2024

bmorris3 force-pushed the dq-decode-2 branch from a492ceb to 9195c39 Compare April 9, 2024 15:02

bmorris3 changed the title ~~Initial data quality plugin UI~~ Data Quality plugin for Imviz Apr 9, 2024

bmorris3 marked this pull request as ready for review April 9, 2024 15:33

bmorris3 requested review from rosteen, javerbukh, pllim, haticekaratay, cshanahan1 and gibsongreen as code owners April 9, 2024 15:33

javerbukh reviewed Apr 11, 2024

View reviewed changes

jdaviz/configs/default/plugins/data_quality/data_quality.py Outdated Show resolved Hide resolved

bmorris3 force-pushed the dq-decode-2 branch from 9195c39 to dfbf46f Compare April 15, 2024 15:32

kecnry reviewed Apr 15, 2024

View reviewed changes

kecnry reviewed Apr 16, 2024

View reviewed changes

javerbukh reviewed Apr 16, 2024

View reviewed changes

bmorris3 added 14 commits April 16, 2024 16:36

adding bit decoding utils

a796f41

working on initial DQ plugin UI

774ab90

adding irrelevant message, commented out

0b0776a

working single-bit masking

8e60870

adding multiselect dropdown bit filter

724a5d5

Imviz DQ parser defaults, fix association visibility logic, add conce…

b01ecd9

…pt nb

add DQ opacity slider, better top-layer filter

b590458

add DQ value to coords-info

7ef0c4a

simplify get_top_layer_index to use viewer reference to app

fa825f0

Parser fix

0f5fa8c

adding DQ plugin tests

2906c91

from review comments by Kyle

f929431

adding public attributes on DQ plugin

1a9f202

more review comments from Kyle

ef295a4

bmorris3 force-pushed the dq-decode-2 branch from 2027888 to 26f59a1 Compare April 16, 2024 20:37

javerbukh reviewed Apr 17, 2024

View reviewed changes

fixes from Jesse's review

a6de416

bmorris3 force-pushed the dq-decode-2 branch from 26f59a1 to a6de416 Compare April 17, 2024 13:46

kecnry approved these changes Apr 17, 2024

View reviewed changes

javerbukh approved these changes Apr 17, 2024

View reviewed changes

Making DQ plugin irrelevant by default while in development

a7e748b

bmorris3 merged commit a9c475c into spacetelescope:main Apr 17, 2024
13 of 14 checks passed

bmorris3 mentioned this pull request Apr 18, 2024

DQ support for Cubeviz #2817

Merged

9 tasks

bmorris3 added a commit to bmorris3/jdaviz that referenced this pull request Jul 17, 2024

fix for Imviz parser romandeps test since default update in spacetele…

5bea741

…scope#2767

bmorris3 mentioned this pull request Jul 17, 2024

Fix for Imviz parser romandeps test after update to defaults #3098

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data Quality plugin for Imviz #2767

Data Quality plugin for Imviz #2767

bmorris3 commented Mar 22, 2024 •

edited

Loading

codecov bot commented Mar 22, 2024 •

edited

Loading

kecnry Mar 25, 2024

bmorris3 Mar 25, 2024

kecnry Apr 15, 2024

kecnry Apr 16, 2024 •

edited

Loading

bmorris3 Apr 17, 2024

bmorris3 commented Mar 27, 2024 •

edited

Loading

javerbukh commented Mar 27, 2024

camipacifici commented Mar 27, 2024

bmorris3 commented Mar 27, 2024

bmorris3 commented Mar 28, 2024 •

edited

Loading

camipacifici commented Mar 28, 2024

bmorris3 commented Mar 28, 2024

bmorris3 commented Mar 28, 2024

bmorris3 Apr 8, 2024

javerbukh Apr 11, 2024

kecnry Apr 11, 2024

bmorris3 Apr 8, 2024

javerbukh commented Apr 11, 2024

bmorris3 commented Apr 15, 2024 •

edited

Loading

kecnry left a comment

bmorris3 commented Apr 15, 2024

kecnry Apr 16, 2024

bmorris3 Apr 16, 2024

kecnry Apr 16, 2024

kecnry Apr 16, 2024 •

edited

Loading

kecnry Apr 16, 2024

bmorris3 Apr 16, 2024

javerbukh left a comment

javerbukh left a comment

javerbukh left a comment

bmorris3 commented Apr 17, 2024

		# TODO: uncomment this line before merging into main:
		# irrelevant_msg = Unicode("Data Quality plugin is in development.").tag(sync=True)

		def layer_is_not_dq(data):
		return not data.label.endswith('[DQ]')


		dq_layer.state.alpha = self.dq_layer_opacity

		def update_visibility(self, index):

Data Quality plugin for Imviz #2767

Data Quality plugin for Imviz #2767

Conversation

bmorris3 commented Mar 22, 2024 • edited Loading

Demo video

Example notebook

Other notes

Tickets addressed

To do list

Change log entry

Checklist for package maintainer(s)

codecov bot commented Mar 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kecnry Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmorris3 commented Mar 27, 2024 • edited Loading

javerbukh commented Mar 27, 2024

camipacifici commented Mar 27, 2024

bmorris3 commented Mar 27, 2024

bmorris3 commented Mar 28, 2024 • edited Loading

camipacifici commented Mar 28, 2024

bmorris3 commented Mar 28, 2024

bmorris3 commented Mar 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javerbukh commented Apr 11, 2024

bmorris3 commented Apr 15, 2024 • edited Loading

kecnry left a comment

Choose a reason for hiding this comment

bmorris3 commented Apr 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kecnry Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javerbukh left a comment

Choose a reason for hiding this comment

javerbukh left a comment

Choose a reason for hiding this comment

javerbukh left a comment

Choose a reason for hiding this comment

bmorris3 commented Apr 17, 2024

bmorris3 commented Mar 22, 2024 •

edited

Loading

codecov bot commented Mar 22, 2024 •

edited

Loading

kecnry Apr 16, 2024 •

edited

Loading

bmorris3 commented Mar 27, 2024 •

edited

Loading

bmorris3 commented Mar 28, 2024 •

edited

Loading

bmorris3 commented Apr 15, 2024 •

edited

Loading

kecnry Apr 16, 2024 •

edited

Loading