nion-software / nionswift-instrumentation-kit

Base classes for Nion Swift STEM microscope instrumentation
GNU General Public License v3.0
1 stars 12 forks source link

Don't ignore metadata coming from camera. #60

Closed Brow71189 closed 3 years ago

Brow71189 commented 3 years ago

Right now any metadata coming from the camera (i.e. in the "hardware_source" group) will be overwritten by the SI code in scan_base.py. It gets replaced with the default metadata retrieved before the SI starts. If the camera wants to add metadata that is not known beforehand it cannot place it into the standard group. An example for metadata that only comes up during the acquisition would be saturated pixels. Out metadata is not standardized very well anyway, but at least we should stick to the (few) standards we already have. Specifically this means that camera metadata should go into the "hardware_source" group, so using a different group is not a good workaround in my opinion. The "scan" and "instrument" groups can probably stay because there shouldn't be any metadata for this group coming from the camera.

cmeyer commented 3 years ago

Before merging this, I'd like to have some time to think about it and a bit more discussion.

Can you open an issue which states the goals of this change: Allow camera to provide metadata that gets stored in final image during SI. Then we can discuss implementation ideas.

Do we need to be able to distinguish between metadata associated with the camera itself, sequences of data from the camera, and synchronized data from the camera? Also, how does processing (projection or summing) have an effect on metadata, e.g. are saturated pixels useful (or accurate) when the data has been summed or projected?

And... I'd like to not perpetuate the use of "hardware_source". It was an early hack and my goal is to replace it with something more descriptive ("detector"?) eventually - so maybe a good fix for this could prepare for that issue too.

Brow71189 commented 3 years ago

I created the issue: #61

Can we just merge this in the meantime anyway? Right now we simply loose all metadta coming from cameras in SI/4D mode.

Do we need to be able to distinguish between metadata associated with the camera itself, sequences of data from the camera, and synchronized data from the camera?

Yes, I think this would be useful. In a sequence/synchronized data there might be metadata that is associated with only one frame in the sequence (for a sequence of n frames you would have n metadata entries for that key). And of course there is also metadata that applies to the camera in general.

Also, how does processing (projection or summing) have an effect on metadata, e.g. are saturated pixels useful (or accurate) when the data has been summed or projected?

Since the camera provides this metadata, it will make sure that it is applicable. And I think metadata is always useful, even if it is not entirely accurate. This habit of ours to just cleanse data of any metdata just because we are not sure that it is entirely accurate is a really bad one in my opinion. See also this related issue about throwing away metadata: https://github.com/nion-software/niondata/issues/16

cmeyer commented 3 years ago

Thanks for this patch. I made some modifications and merged it fc30532fc78783d655c305845eb6fc3310c870e8. I added a note about future behavior - the important part: write a test for this behavior so if it's changed in the future, the test fails.