Add external mask parameter to MakeMask

[mcb64]

Allow MakeMask to accept an extMask parameter which, if not None, is the properly-shaped mask to create and apply.

This requires knowing the shape of the raw data, so the rawShape property was added to the detector object. This is initialized by the _getRawShape method which is called after any data retrieval.

[mcb64]

One minor issue I have with this as is if you save a mask made through the regular loop and try to use it as an extMask, you have to ask for it to be inverted. Because "inverted" is actually "not inverted":

    if raw_input("Invert [y/n]? (n/no inversion: masked pixels will get rejected)?\n") in ["y","Y"]:
        totmask = (totmask.astype(bool)).astype(int)
    else:
        totmask = (~(totmask.astype(bool))).astype(int)

[silkenelson]

I would have preferred to use the mask as it is used when defining a masked array (or how a physical carnival mask works): non-zero entries mark where something in the data should be masked. This is how the mask is built originally.

But psana.Detector.mask(....) returns the inverse: zero means a pixel is masked. I assume that e.g. for common mode or similar calculations, the data array is multiplied with the mask. I have chosen to have DetObject.mask mirrors this behavior which means that I have to invert. I am very open to at least improve the verbage used in the questions and/or add some text that clarifies this, in particular when proving a user-created mask.

This is largely under the hood and could be changed, but I think it would be more confusing if the mask returned by smalldata/DetObject were inverted from what the psana.Detector returns. I do wish it was an actual mask though.

[mcb64]

extMask is the new parameter to MakeMask that is the enhancement that this PR is all about.

Currently, it's assumed that, as is the case with the masks built within the loop, 1 is a masked pixel, and 0 is let through. Once the mask is ready, MakeMask asks "Do you want to invert this?" and if the answer is yes, it doesn't invert. So the mask returned is one that has 0 for masked pixels and 1 for pixels that should be let through.

If extMask should have 0 for masked pixels, it's easy enough to change what "invert" does here.

This is really a question of what you want extMask to do...

I can change the wording here as well, but I worry about breaking expectations if invert suddenly means really invert it.

[mcb64]

(Also, I guarantee that every computer science guy in the world thinks that 1 is a pixel that goes through.)

[vespos]

Yes the mask definition with 1 = discarded pixel has always seemed odd to me, but this seems to be the standard in some communities.

[silkenelson]

Originally I thought that too, but then think about what e.g. Yanwen had done when they made a mask for the epix: the produce a thing where material was where she did not want signal on the detector....mask==1 -> blocked just work exactly like a physically produced mask.

[mcb64]

I just had a thought... perhaps the thing to do is totally change the question in the extMask case.

Currently the question is: Invert [y/n]? (n/no inversion: masked pixels will get rejected)?

I suggest keeping this, but if an external mask is plugged in, instead ask: Does the external mask have zero values for masked pixels [y/n] (n: non-zero pixels are rejected)?

Thoughts?

[silkenelson]

That seems good.