Detectors do not have pydantic discriminator field

[melisande-c]

• microsim version: 0.0.8.dev3+gcd9ecff

Description

Hi @tlambert03,

I've noticed that your detector classes do not have a discriminator field, like, for example, your modality classes do. I'd like to save the settings that I ran my simulations with, and then potentially load the same settings; it would be convenient to just use the pydantic model_dump method, but the lack of a detector discriminator complicates things slightly.

The following code snippet demonstrates what I mean. When I load the output of model_dump, the detector type is not the original that I chose. I assume the resulting camera type is the first that pydantic finds.

from pprint import pprint
import microsim.schema as ms

ground_truth_shape = (32, 64, 512)
downscale = 4

sim = ms.Simulation(
    truth_space=ms.ShapeScaleSpace(
        shape=ground_truth_shape, scale=(0.02, 0.01, 0.01)
    ),
    output_space={"downscale": downscale},
    sample=ms.Sample(labels=[ms.MatsLines()]),
    modality=ms.Widefield(),
    detector=ms.CameraCCD(qe=1, read_noise=4, bit_depth=12, offset=100),
)
settings = sim.model_dump()
print("Resulting detector type: ", type(ms.Simulation(**settings).detector))
print("Simulation model dump:")
# print without sample and channels to shorten output
pprint(sim.model_dump(exclude=["sample", "channels"]))

Output:

Resulting detector type: <class 'microsim.schema.detectors._camera.CameraCMOS'>
Simulation model dump:
{'detector': {'bit_depth': 12,
              'clock_induced_charge': 0.0,
              'dark_current': 0.001,
              'full_well': 18000,
              'gain': 1.0,
              'name': '',
              'offset': 100,
              'qe': 1.0,
              'read_noise': 4.0,
              'readout_rate': 1.0},
 'exposure_ms': 100.0,
 'modality': {'type': 'widefield'},
 'objective_lens': {'coverslip_ri': 1.515,
                    'coverslip_ri_spec': 1.515,
                    'coverslip_thickness_spec_um': 170.0,
                    'coverslip_thickness_um': 170.0,
                    'immersion_medium_ri': 1.515,
                    'immersion_medium_ri_spec': 1.515,
                    'magnification': 1.0,
                    'numerical_aperture': 1.4,
                    'specimen_ri': 1.47,
                    'working_distance_um': 150.0},
 'output_path': None,
 'output_space': {'downscale': 4,
                  'reference': {'axes': (<Axis.Z>, <Axis.Y>, <Axis.X>),
                                'extent': (0.64, 0.64, 5.12),
                                'scale': (0.02, 0.01, 0.01),
                                'shape': (32, 64, 512)},
                  'scale': (0.08, 0.04, 0.04),
                  'shape': (8, 16, 128)},
 'settings': {'binning_strategy': 'equal_space',
              'cache': {'read': True, 'write': True},
              'device': 'auto',
              'float_dtype': dtype('float32'),
              'max_psf_radius_aus': 8.0,
              'max_wavelength': 800,
              'min_wavelength': 300,
              'np_backend': 'auto',
              'num_wavelength_bins': 32,
              'random_seed': 1869206325,
              'spectral_bin_threshold_percentage': 1.0,
              'spectral_bins_per_emission_channel': 4},
 'truth_space': {'axes': (<Axis.Z>, <Axis.Y>, <Axis.X>),
                 'extent': (0.64, 0.64, 5.12),
                 'scale': (0.02, 0.01, 0.01),
                 'shape': (32, 64, 512)}}

Thanks ☺️

[tlambert03]

let's go ahead and add one! In practice, I suspect that once CameraCMOS is fully implemented, then there will be effectively no overlap between the definitions of the three main types of cameras (CCD, EMCCD, and CMOS)... because CMOS will gain additional properties that CCD do not have (and may lose some that CCDs do have). So, at the moment, this is happening because CMOS isn't actually implemented, and that smart union is just making it to the CameraCMOS in the Detector Union and finding it satisfied (it is satisfied at the moment because CMOS is still at this point literally just the same thing as CCD)

TLDR:

• in terms of simulation output, your use case is fine for now, and probably in the future as well. for now CMOS is essentially the same thing as CCD, and when that changes, your serialized object would no longer register as a CMOS.
• it would be absolutely fine to add a discriminator field anyway, just to be explicit

[melisande-c]

Ok great! I was uncertain about it because the binning is different for the CMOS camera in the simulate_camera function here:

microsim/src/microsim/schema/detectors/_simulate.py

Lines 82 to 84 in 8f1ee38

	# sCMOS binning
	if binning > 1 and isinstance(camera, CameraCMOS):
	gray_values = bin_window(gray_values, binning, "mean")

[tlambert03]

Ah indeed, that would make a difference for anything with a binning != 1. All the more reason to add the discriminator. Thanks for noting that

[tlambert03]

Probably would have been more obvious if the simulate_camera function were a method on these cameras. So perhaps that would also be a good change here

[melisande-c]

I can make a PR to add the discriminator if you like, but I'll probably leave the refactoring work to move the simulate_camera function to the camera classes.

Couple of questions:

• I noticed there is an unused name field in the base Camera class, should it be used for the discriminator?

  microsim/src/microsim/schema/detectors/_camera.py

  Line 29 in 8f1ee38

  name: str = ""

• For the detector names, would you prefer all snake casing, like: "camera_cmos", or capitals for the acronyms, like: "camera_CMOS"? Or anything else?

[tlambert03]

thanks @melisande-c, I did the method refactor in #91, and added the camera_type union field while I was at it. The name field, by the way, is just a descriptor to be used by the camera "preset" library (like ICX285)

[melisande-c]

Ah amazing! Thanks!