micro-manager / mmCoreAndDevices

Micro-Manager's device control layer, written in C++
40 stars 105 forks source link

Add utility for storing custom image metadata #329

Open ieivanov opened 1 year ago

ieivanov commented 1 year ago

I plan to create a utility device adapter which will store custom image plane metadata. When the device is initialized, it will ask the user how many custom metadata fields they would like to create, and how they should be named. Initially, it will only support storing string metadata, but this may be extended in the future. Custom metadata can then be entered manually through the device property browser or programmatically using mmc.setProperty('MetadataTracker', 'Metadata_field_name_1', 'metadata_value'). The metadata fields can also be exposed in the GUI as a single-property config group for ease of use (for example the way we currently expose laser intensity).

This will allow us to store the state of non-motorized components, or other microscope properties, such as condenser NA. This will also allow us to easily link image data with calibration metadata applicable across multiple acquisitions. And it will alleviate the need for storing extra metadata in the acquisition name.

@nicost @henrypinkard @marktsuchida this seems generally useful, and I'm surprised something like this doesn't exist already. Or am I missing something?

cc: @talonchandler @edyoshikun @mattersoflight

henrypinkard commented 1 year ago

This use case makes sense to me. I think you can do a version of this currently with AcqEngJ using image processors to add metadata tags as you wish, but this wouldn't already be present in TaggedImages that come directly from the core.

I think people do versions of this already by including demo devices in their config. For example, a demo objective is part of the pixel size config if you don't have a motorized switcher. You're idea seems like a more general and potentially more useful version of this.

So I'm guessing you'll have a pre-init property in the DA that you set something like propertyNames to propName1,propName2 etc, and then dynamically create those at startup?

nicost commented 1 year ago

Sorry for not responding earlier. Currently, all properties known to the Core are added to image metadata, so I do not fully understand how your ask differs from the existing property mechanism. Any device adapter creating properties can be used for what you describe. May be you would like a device adapter that can create properties based on pre-initialization properties? Or a device adapter that creates properties based on the contents of a file it reads?

ieivanov commented 1 year ago

Thanks for your reply Nico!

Here I'm suggesting creating a utility device adapter which can capture arbitrary metadata, for example, the name of the microscope used for acquisition. Such metadata is often included in the dataset file name, but that is up to the user to properly include in a standard format. The value of this utility is that this metadata can be included in the microscope config file and automatically written in tagged images. Such fields can be read-only after init. This utility can also provide a way to capture information about non-motorized components of the microscope, say the size of the condenser aperture stop. These fields can be editable.

Does this make sense to you?

nicost commented 1 year ago

What should that look like? You could make several pre-init properties, but then the names of those pre-init properties are hard coded. I kind of like the idea of a device adapter that reads in a file that contains key-value pairs, such as: MicroscopeName, CoolBird CondenserNA, 0.54 Location, BioHub-365

where all of these are then made into read-only properties. The location of the file could be set as a pre-init property.

Does that make sense (and do what you like)?

henrypinkard commented 1 year ago

I kind of like the idea of a device adapter that reads in a file that contains key-value pairs, such as: MicroscopeName, CoolBird CondenserNA, 0.54 Location, BioHub-365

where all of these are then made into read-only properties. The location of the file could be set as a pre-init property.

It would be nice to take this a step further put everything into the existing config file. Would it make sense to do this all with pre-init properties that define the read-only properties?

PropNames: "MicroscopeName-String-CoolBird;CondenserNA-Float-0.54"

Then when the device adapter initializes, It creates all these as readonly props

nicost commented 1 year ago

I can see that it will be nice to include these in the config file (although it seems more important to have these in the images). Setting these as pre-init properties has the downside that you have to decide in advance how many of these to make available. Also, the required input format is quite complicated and error prone.

Another idea (not that I am convinced it is a good one) would be to give the upper Core API the ability to create (read only) Core properties. That would make it possible to add properties at will, and these could be specified in the configuration file. Sounds decent while I am writing this;)

henrypinkard commented 1 year ago

I can see that it will be nice to include these in the config file (although it seems more important to have these in the images).

Whether theyre stored in config file or extra file, they both end up in the image metadata, no?

Setting these as pre-init properties has the downside that you have to decide in advance how many of these to make available. Also, the required input format is quite complicated and error prone.

I don't think so, because you can parse a single string with them all listed together. A better way that would be less complicated and error prone would be to have one pre-init property corresponding to each type (String, float, etc) and then have them list the names of the properties. You just need a separator (like :)

// create 3 string props
StringProps: "Beak:Feathers:Wings"
// create 1 float prop
FloatProps: "CondenserNA"

Another idea (not that I am convinced it is a good one) would be to give the upper Core API the ability to create (read only) Core properties. That would make it possible to add properties at will, and these could be specified in the configuration file. Sounds decent while I am writing this;)

Seems like a reasonable addition. Though for this use case to me it seems preferable to have a mechanisms to have the information in some sort of file, since it sounds like this is for static information about the microscope (is that right @ieivanov?)

nicost commented 1 year ago

Seems like a reasonable addition. Though for this use case to me it seems preferable to have a mechanisms to have the information in some sort of file, since it sounds like this is for static information about the microscope (is that right @ieivanov?)

With this mechanism, the properties would end up in the config file. The config file parser would know to create Core properties, and set them to the desired value. This would need to be done in two places (C++ layer, which reads in the config file), and the Config Wizard code, which also reads in the config file.

I am not a big fan of the long String that contains several magic characters to be parsed into several units. Very easy to get that wrong.

henrypinkard commented 1 year ago

With this mechanism, the properties would end up in the config file. The config file parser would know to create Core properties, and set them to the desired value. This would need to be done in two places (C++ layer, which reads in the config file), and the Config Wizard code, which also reads in the config file.

Oh are you thinking that these have to be properties of the Core? Not of the device adapter itself?

If its just a regular device adapter, seems to me there wouldn't need to be any changes to the core or config wizard

I am not a big fan of the long String that contains several magic characters to be parsed into several units. Very easy to get that wrong.

There's only one magic character (: in my example), and if the magic character character were a line break than this would just be a regular list in a text file:

StringProps: 
   Beak
   Feathers
   Wings

Whether it goes in the config file or a different text file, I don't see how you can avoid having a list of things with a special character separating them

ieivanov commented 1 year ago

Thanks guys, I think we are converging on something here.

My initial intent was for this utility to log both static and dynamic metadata properties. The dynamic properties may be changed by the user after init. The workflow I am imaging would be for the user to select how many fields they want, what they should be called, whether they should be real-only, and what value they may have (optional for editable fields), during init of the device adapter with the config wizard. All this information will then be saved in the microscope config file .cfg as pre-init settings. And loaded again next time micro-manager is launched.

I am not sure how practical this would be to develop, happy to follow your advice here. Could we use the hub API if necessary to deal with the variable number of device properties with custom names?

marktsuchida commented 1 year ago

To me this looks a little similar to what we already have in DemoCamera's DStateDevice (which is sometimes used for purposes similar to this, such as recording the position of a manual filter switcher). That device allows you to set the number of positions (which can be 1), and (being a state device) each position can be labeled by the user.

Of course the further customizability that @ieivanov is proposing is quite lacking from DStateDevice.

Also, to create multiple copies of DStateDevice you need multiple copies of DHub, which may not be ideal (but see below).

So here are my thoughts on solutions that do not require modifying the config file format, yet allows all the information to live in the config file -- similar to what @ieivanov is getting at, I think. What follows does seem to make it clear that at least one setting will need to contain a delimiter-separated list. However, there is no need to make it any more complicated than that.

Given the constraints on pre-init properties and the hub-peripheral model, I think there are roughly 3 possible approaches:

  1. Do not use a hub; allow multiple copies of the device but only 1 field (property) per device.
  2. Use a hub; allow multiple copies of the device per hub.
  3. Use a hub; allow a single copy of the device per hub, but multiple hubs can be used.

Approach 1 is the simplest; the device could have the following pre-init properties, which closely match how device properties behave:

(Note that commas are not allowed in property names or values (due to the .cfg format). In this case, semicolons would also be unavailable, or an escape sequence could be parsed, but I would vote for just disallowing. Semicolons are generally less dangerous than colons on Windows, when considering that property values have been known to end up in strange places.)

If we take approach 2, the hub must have a pre-init property specifying the number of devices to create. If allowing each device to have more than one field, the hub would need to have a pre-init property that is a list of field counts.

In approach 3, the hub's pre-init properties can be used essentially as pre-pre-init properties of the device, allowing one additional level of configuration. So the hub could specify the number of device fields, and then the device can create its own pre-init properties (similar to those I listed above) for each field: Field1IsReadOnly, Field2DataType, etc.

One slight complication is that pre-init properties need to be created in the device's constructor, where only the name of the device is available (access to the hub becomes available in Initialize()). So the number of fields will need to be encoded into the device name. This applies to approach 2, too, if allowing multiple fields per device.

At the moment I can't think of any clear argument against any of the 3 approaches. And a few other variants are probably also possible.

I would prefer that this be a new, separate device adapter if introducing a hub device. If taking approach 1, it could potentially be part of Utilities.

Also, if only allowing 1 field per device, it may make sense to make it a state device and reuse the Label mechanism, at least when it is read-write and has discrete values (but one could also argue against this for symmetry with the read-only or continuously varying cases). If allowing multiple fields per device, it should probably be a "generic" device.

To clarify, I'm also not against the idea of having a completely separate config file dedicated to this device type only. Then configuration would be extremely simple (just specify the file path as a pre-init property). The main tradeoff is that the above approach (based purely on properties) travels with the .cfg file but is tedious to configure/edit (at least without additional tools), while the separate file approach is easier to edit but users need to ensure the file is in the right place. One question, then, is how many of these we expect people to use. I think there is a significant advantage to have everything contained in the .cfg if we are talking about a handful of properties. If we are talking about dozens, we probably need either a separate file or some kind of supporting tools (or import/export commands).