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

Need per-image pixel mask #651

Closed
phyy-nx opened this issue Mar 26, 2019 · 16 comments
Closed

Need per-image pixel mask #651

phyy-nx opened this issue Mar 26, 2019 · 16 comments
Labels
Milestone

Comments

@phyy-nx
Copy link
Contributor

phyy-nx commented Mar 26, 2019

The NXmx specification of pixel_mask is:

pixel_mask[i, j, k]: (optional) NX_INT

This specifies a single mask applicable to all images in the dataset. This is a valid use case. I believe it could also be defined as:

pixel_mask[np, i, j, k]: (optional) NX_INT

i.e., it is valid for the user to provide a separate mask for each image. This is the case for the EuXFEL.

Therefore, the pixel mask always has a number of dimensions equal to the dimensionality of the data OR the dimensionality of the data -1, where the former is for a mask for each image and the later is a mask for the whole dataset.

Thoughts?

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 26, 2019

@graeme-winter points to a similar discussion about shadow masks in the HDRMX working group:
https://github.com/HDRMX/NXmx/wiki/Treatment-of-(Dynamic)-Shadows-in-Eiger-Data-Sets

I like the delineation of static mask vs. dynamic mask. Might be better than having it implicitly baked into the dimensions of pixel_mask...

@zjttoefs
Copy link
Contributor

zjttoefs commented Mar 27, 2019 via email

@yayahjb
Copy link
Contributor

yayahjb commented Mar 27, 2019 via email

@graeme-winter
Copy link
Contributor

I can see an argument for dynamic and static masks explicitly, in that you could be using both: a backstop / bad pixel mask which is static and a second computed mask which handles, in this example, the shadow. The latter could be interpreted as a suggestion or guide, or best effort. Keeping these separate has value to me...

@zjttoefs
Copy link
Contributor

zjttoefs commented Mar 27, 2019 via email

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 27, 2019

Keeping them separate though lets us do things like hard linking to a never-changing known static mask (the pixel_mask) while also generating a runtime specific mask (the dynamic_mask).

@zjttoefs
Copy link
Contributor

zjttoefs commented Mar 27, 2019 via email

@rayosborn
Copy link
Contributor

We use both static and dynamic masks in our single crystal diffuse scattering code. The argument for keeping them separate is that you may need to regenerate the dynamic mask as you refine your analysis. For example, we use dynamic masks to eliminate Compton scattering halos around Bragg peaks, generating them by an algorithm that calculates the Bragg peak positions. If we discover that our assumed space group was wrong, we might need to regenerate the mask, but we would not then have a record of the bad detector pixels.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 27, 2019

I think @zjttoefs is saying that record could persist through the use of bit fields. bit 8: user defined mask (e.g. around beamstop) vs bit 1: dead. Right?

@zjttoefs
Copy link
Contributor

zjttoefs commented Mar 27, 2019 via email

@yayahjb
Copy link
Contributor

yayahjb commented Mar 27, 2019 via email

@graeme-winter
Copy link
Contributor

Folks - in discussion with @phyy-nx yesterday I was reminded of another motivator for the two masks - the pixel mask as traditionally defined (i.e. 2D version) is available directly from the camera and is known at acquisition time - the shadow / dynamic mask may only be computed a postori or at the same time as the data acquisition, so could be linked into the master file in anticipation of the existence of the file.

At the moment (see HDRMX link above) the calculation of the mask takes longer than acquiring the images... this is also advisory and could be refined so I do think there is a good argument for separate mask data sets.

phyy-nx added a commit that referenced this issue Mar 29, 2019
Also sync documentation between NXmx and NXdetector.
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 29, 2019

I've added a new pull request, #655, that uses a dynamic mask instead of making pixel_mask mult-image (as in #654). I'm happy with either #654 or #655 as solutions, but @graeme-winter has shown use cases where as separate mask is preferable. Other comments?

@yayahjb
Copy link
Contributor

yayahjb commented Mar 29, 2019 via email

@graeme-winter
Copy link
Contributor

There is a specific use case where the dynamic mask will be computed at the same time as data are acquired, and referenced from the master file. In this case there will be a need for two pixel masks, one 2D one from the detector and a second which will appear later which is 3D. Avoids holding up acquisition for the shadow calculation...

@prjemian prjemian added this to the NXDL 2019.3 milestone Apr 2, 2019
@phyy-nx phyy-nx closed this as completed in 4925eeb Apr 3, 2019
phyy-nx added a commit that referenced this issue Apr 3, 2019
Allow multi-image masks. Resolves #651.
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Apr 3, 2019

Leaving this open as there's still a desire to add a separate mask, dynamic_mask (#655). Probably best to discuss at next telco.

@phyy-nx phyy-nx reopened this Apr 3, 2019
phyy-nx added a commit that referenced this issue Jul 30, 2019
Also sync documentation between NXmx and NXdetector.
@phyy-nx phyy-nx closed this as completed in 6d0c900 Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants