Raster Scanning Code by GlennZZY · Pull Request #55 · siplab-gt/cleo

GlennZZY · 2024-11-15T03:01:45Z

Uploaded the code for raster scanning.

Since there's only one registry for simulation, the flag might not work at this moment

kjohnsen · 2024-11-20T16:55:22Z

        factory=list, repr=False, init=False
    )
    """relative expression levels of neurons selected from each injection"""
+    Registry: DeviceInteractionRegistry = None


Are you sure we need to store the registry when we can just do registry_for_sim(self.sim)?

kjohnsen · 2024-11-20T20:20:09Z

        self.focus_coords_per_injct.append(focus_coords)
        self.rho_rel_per_injct.append(rho_rel)
+        self.Registry = registry_for_sim(self.sim)
+        self.Registry.update_fov(self.img_width)


would this need to be updated on every injection? Also, it can't be right that there's a single update_fov for the whole registry, since this should vary for each light device

the Scope should optionally have an imaging_light object, and the img_width and scan_freq would be updated specific to that light

kjohnsen · 2024-11-20T21:44:49Z


+    raster_fov: int = field(default=500 * 1e-6 * meter, kw_only=True)
+
+    raster_enable: int = field(default=0, kw_only=True)


again, these would be at the level of individual lights, not the whole registry

this is still the case. These would be Brian state variables inside the Light neuron group

kjohnsen · 2024-11-20T21:49:26Z

    wavelength: Quantity = field(default=473 * nmeter, kw_only=True)
    """light wavelength with unit (usually nmeter)"""

+    scan_freq : int = field(default=30, kw_only=True)


I'd create a subclass RasterScanningLight to hide the scan_freq details from the typical Light object

kjohnsen · 2024-11-20T21:54:20Z

        i_source = self.subgroup_idx_for_light[light]
        light_prop_syn.epsilon[i_source, :] = epsilon
        light_prop_syn.T[i_source, :] = light.transmittance(coords_from_ng(ng)).ravel()
+        light_prop_syn.scan_period = second / light.scan_freq


I'd put these in a separate function that you call both here (on injection) and when you change the params on the scope/imaging light (setter functions). like update_raster_scan_params(scope) or something like that

like you've already done with update_fov but can include these other variables too

kjohnsen

Looks pretty good! Besides the comments below, as I was about to test it (with the script at the bottom that plots all the action spectra), it occurred to me it's getting to be too much to check just by looking at the numbers. I think we need a new tutorial notebook that goes over all the opsins that will serve two puposes: (1) quickly introduce the user to all the options and (2) let us visually inspect that all the opsins are behaving as expected. Can you create a docs/tutorials/opsin_selection.ipynb notebook where we plot 4 things for every opsin?

The 1P action spectrum,
the 2P action spectrum,
the impulse response (the photocurrent after a quick (e.g., 5 ms), strong pulse),
and the pulse response (what the photocurrent looks like with prolonged stimulation (should see peak, then back down to plateau)

Since there are now too many opsins to visualize all at once, can you break this into groups? For example, plot all the ChR2 variants together, all the Chrimson variants together, all the inhibitory pumps together, all the ChRmine variants together?

kjohnsen · 2025-04-02T20:59:19Z

can we remove these? we should add **/.DS_Store to .gitignore

kjohnsen · 2025-04-02T22:04:27Z

 "metadata": {
  "kernelspec": {
-   "display_name": "python3",
+   "display_name": ".venv",


no need to commit this notebook if you didn't change it. Also, if you did change the notebook, use nbdev clean to get rid of stuff like this before committing

kjohnsen · 2025-04-03T13:26:42Z

+            (490, 0.92),
+            (498, 1),
+            (516, 0.84),
+            (507, 0.90),


here's another place the spectrum is out of order. For legibility and reproducibility, it should be in order, but I remembered I actually should have already accounted for this in light_dependence.py, line 125...don't know why I'm getting an error. Could you add a unit test for this? I guess it would be a new file tests/light/test_light_dependence.py

kjohnsen

The overall pattern of creating a light from the scope looks good! Just time to put it all together, make some tweaks, and most importantly, get it working in an end-to-end example and in unit tests

kjohnsen · 2025-07-25T18:05:54Z

    def init_for_simulator(self, sim: CLSimulator) -> None:
        registry = registry_for_sim(sim)
        registry.init_register_light(self)
+        """Test Code for setter method"""


I assume you'd move this to test_scope.py?

… default 1p/2p ratio in opsin library to be average of other opsin ratios.

…n group, fix registry rebuild, wire pulse_stagger, add unit tests

… default 1p/2p ratio in opsin library

kjohnsen

Good progress. Quite a bit of changes to make though. Summary of the main architectural change:

Variables that live on scope (attrs fields)

img_width
t_flyback

properties/setters on scope

scan_freq → pulse_freq on light
- which in turn is property/setter to light.source neurons
dwell_time: read-only, i.e., property only, not a setter → light.pulse_width
- which in turn is property/setter to light.source.pulse_width

properties/setters on light, pointing to light.source Brian variables

we don't expose these to the user on scope since they'd always be True
- pulse_stagger
- is_scanning (rename to is_pulsed)
these get called from different names on scope, as noted above
- pulse_width
- pulse_freq

For user-friendliness, since there'd no longer be fields on Scope with scanning variables scan_freq or t_flyback (I guess it's just those two, right?), we'd make these more discoverable to the user by making them documented keyword arguments of the inject_imaging_light_for_targets() function. img_width would already be defined, is_pulsed would automatically be true, and dwell_time and pulse_stagger would be computed automatically.

Now I was hoping to let the Brian light.source variables and equations be the single source of truth, but we have a mix of variables defined on the scope (like img_width) and ones defined on the light. So we do need something to keep the light variables in sync when img_width, t_flyback, or scan_freq are changed on scope.

To get that setter behavior with attrs fields, I recommend using on_setattr instead of the _field style you used:

import attrs

def sync_nested_c(instance, attrib, new_value):
    if hasattr(instance, "b") and hasattr(instance.b, "c"):
        instance.b.c.att = new_value
    return new_value

@attrs.define(on_setattr=sync_nested_c)
class Container:
    att: str = attrs.field(
        converter=str.strip,
        validator=attrs.validators.instance_of(str),
    )
    b: 'Containee'

I'd write a single function that computes dwell time, effective power, etc., and updates it all on the light (which in turn uses its setters to update on neurons) and use it with on_setattr as well as setters for the non-scope variables.

kjohnsen · 2026-07-27T13:01:17Z

        self.focus_coords_per_injct.append(focus_coords)
        self.rho_rel_per_injct.append(rho_rel)
-        registry_for_sim(self.sim).update_fov(self.img_width)
+        if self.imaging_light is not None:


do we support separate img_width per injection? I don't think so, right? So the point here is to just to make sure that if the light is new, that img_width gets set properly? In that case, we'd have other variables that need to get set on the light, like frequency, width, power

Actually, since we are taking the shortcut of asking the user to inject the sensor after injecting into target neurons, I think we'd want to do the same with the light. In which case, we'd never have a light already created when we're injecting the scope into neurons

kjohnsen · 2026-07-29T14:32:00Z

+
    # Method for optional raster scanning
-    def create_imaging_light(self):
+    def create_imaging_light(self, wavelength=920e-9 * meter, pulse_freq=30):


I think we said we wanted to keep the light tucked away, controlled by scope. So probably inject_imaging_light_for_targets() instead, and inject directly here

The Gemini chat talks about how to calculate the duty cycle/dwell time more accurately

kjohnsen · 2026-07-29T15:14:21Z

    """light wavelength with unit (usually nmeter)"""

-    scan_freq : int = field(default=30, kw_only=True)
+    _pulse_freq: float = field(default=30, kw_only=True, alias="pulse_freq")


rather than store these as fields on the scope, let's let them be purely derived from the Light. so in the @property, it'd be sth like return self.imaging_light.source...

So while the interface lives here on scope, the single source of truth would be the light itself (and for that, it will often be the source neurons that instantiate the light, so another layer of properties where needed)

and remember from the point of view of imaging, we want to call this scan_freq, then under-the-hood what gets changed on the light is pulse_freq and pulse_width. The latter of which is calculated from dwell time, which is calculated from img_width, pulse_freq, soma_radius, and t_flyback

kjohnsen · 2026-07-30T12:24:36Z

+    sim.inject(light, ng)
+    syn = list(sim.registry.light_prop_syns.values())[0]
+    print(list(syn.variables.keys()))
+    scope.is_scanning = True


wouldn't want setter here; would always use scanning 2p light

kjohnsen · 2026-07-30T12:26:10Z

+    if not is_scanning:
+        return
+
+    # check at multiple timepoints across one scan period


this doesn't test that the actual package code is working, but rather that the equations here are working. need to actually run a mini-simulation and check results

kjohnsen · 2026-07-30T12:28:12Z

+    src = sim.registry.source_for_light(light)
+    print(list(src.variables.keys()))
+
+    light.pulse_freq = 50


these should probably be tested in test_light.py
with the changes accounting for dwell time, this tests will be trickier. will need to set scan_freq on scope. then dwell time will be computed from img width, flyback, soma radius, etc. to then set pulse_width, like mentioned previously

kjohnsen · 2026-07-30T12:29:18Z

 from cleo.opto import Opsin, chr2_4s, vfchrimson_4s
 from cleo.registry import registry_for_sim
+from cleo.imaging.scope import Scope
+


remember to use ruff formatting; should automatically sort/organize imports

kjohnsen · 2026-07-30T12:32:15Z


 # Cython debug symbols
-cython_debug/
+cython_debug/**/.DS_Store


only need the latter one; will ignore anywhere

…moved registry_for_sim lookup

…s_pulsed; add soma_radius field

Create Branch

34465d7

kjohnsen reviewed Nov 20, 2024

View reviewed changes

kjohnsen assigned kjohnsen and GlennZZY and unassigned kjohnsen Nov 20, 2024

randismaael added 2 commits February 14, 2025 16:28

Merge branch 'master' into raster-scanning

25f6828

Opsin Library Update

d9d00ab

kjohnsen requested changes Apr 3, 2025

View reviewed changes

Updated Raster Scanning code

edc09d5

kjohnsen requested changes Jul 25, 2025

View reviewed changes

randismaael added 7 commits February 13, 2026 04:23

Added 1-Photon 2-Photon data for more opsins in notebook, updated the…

3d43c28

… default 1p/2p ratio in opsin library to be average of other opsin ratios.

WIP: refactor create_imaging_light and scan_freq property

0207e8b

Add .DS_Store to .gitignore

701d945

Remove .DS_store files and add to .gitignore

d6b05c3

Refactor raster scanning: pulse train params, scan variables to neuro…

d25565a

…n group, fix registry rebuild, wire pulse_stagger, add unit tests

Added 1-Photon 2-Photon data for more opsins in notebook, updated the…

4211207

… default 1p/2p ratio in opsin library

Fixed opsin library formatting and added opsins to init.

f99d727

kjohnsen requested changes Jul 30, 2026

View reviewed changes

kjohnsen assigned randismaael and unassigned GlennZZY Jul 30, 2026

randismaael added 5 commits September 13, 2026 20:28

Merge branch 'master' into raster-scanning

36000f9

Fix registry access after merge: use sim.registry field instead of re…

fe27bf2

…moved registry_for_sim lookup

Refactor Light scan params to use on_setattr; rename is_scanning to i…

7ddb6e0

…s_pulsed; add soma_radius field

Fix malformed .gitignore entry (merged DS_Store/pkl patterns)

97b6b98

Revert soma_radius to Scope-only field

998d8b4


		raster_fov: int = field(default=500 * 1e-6 * meter, kw_only=True)

		raster_enable: int = field(default=0, kw_only=True)

Conversation

GlennZZY commented Nov 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjohnsen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kjohnsen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kjohnsen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants