AllenNeuralDynamics / dynamic-foraging-task

Bonsai/Harp workflow for Dynamic Foraging with Python GUI for visualization and control
MIT License
5 stars 4 forks source link

Build rig json #447

Closed alexpiet closed 2 months ago

alexpiet commented 2 months ago

Pull Request instructions:

Describe changes:

What issues or discussions does this update address?

Describe the expected change in behavior from the perspective of the experimenter

Describe any manual update steps for task computers

Was this update tested in 446/447?

XX-Yin commented 2 months ago

@alexpiet Thanks for making this rig.json. I think there are repeated efforts. We should talk about how to combine them.

I also developed code to flexibly generate the rig.json (linked below). The logic is devices are shared in the local_devices.py, and we need to list what devices to use in the fields_for_generating_rig_metadata.json, then the generate_rig_metadata.py will generate rig metadata. I have used this code to generate the rig.json for ephys 3. It could be extended to other rigs.

https://github.com/AllenNeuralDynamics/CTLUT-metadata-gen/blob/generate_rig_metadata/src/CTLUT_metadata_gen/generate_rig_metadata.py https://github.com/AllenNeuralDynamics/CTLUT-metadata-gen/blob/generate_rig_metadata/src/CTLUT_metadata_gen/local_devices.py https://github.com/AllenNeuralDynamics/CTLUT-metadata-gen/blob/generate_rig_metadata/src/CTLUT_metadata_gen/fields_for_generating_rig_metadata.json

XX-Yin commented 2 months ago

Can you explain more how do you ensure the rig.json is up to date? The rig.json could be very different in different rigs.

My general opinion is the rig.json should not be created from the behavior GUI, but from an independent source. Reasons are 1) rig.json is independent from the behavior. 2) There are a lot of variations across rigs, which requires providing extra information to create rig.json (my solution is to define local devices, fields to select which devices to use and a script to parse rules).

alexpiet commented 2 months ago

Can you explain more how do you ensure the rig.json is up to date? The rig.json could be very different in different rigs.

My general opinion is the rig.json should not be created from the behavior GUI, but from an independent source. Reasons are 1) rig.json is independent from the behavior. 2) There are a lot of variations across rigs, which requires providing extra information to create rig.json (my solution is to define local devices, fields to select which devices to use and a script to define rules).

I ensure the rig.json is up to date by generating a new rig.json and seeing if anything changed other than the date.

I generate the new rig.json in a very similar manner to how you generate them. Each rig has details provided in ForagingSettings.json, Settings_Box.csv, and then any additional details can be added to rig_specification.json. Those settings, along with any calibration files, are equivalent to the "fields" in your set up, then I have a function that generates the rig.json (equivalent to your local devices and script).

There are two big advantages of generating the rig.json in the GUI. (1) It happens automatically without a user having to generate a new rig.json on every rig. (2) When a user updates the rig, most of those changes are already going to be reflected in ForagingSettings.json, or Settings_Box.csv, so it makes sense to also log them in rig_specification.json.

Any rig can opt-out of this process, and generate their own rig.json. I imagine the Ephys rigs will want to do this since the rig.json will be much more complicated.

XX-Yin commented 2 months ago

I understand you want the rig.json to be automatically generated when changes occur. I agree with this idea. However, this can be an independent process. It will make the maintenance of the code very hard if we integrate "generating rig.json" to the behavior GUI. The main reason is the devices and the code to generate the rig.json are very different across different rigs. If they were all hard-coded in the behavioral GUI, we would need to constantly update the code. For example, there are more fields that need to be added to rig.json (e.g. LED for optogenetics, IR light, patch cable, serial numbers of devices).

Currently, everyone writes their own code to generate rig.json. I think we should have a unified repository to standardize this process. My idea is to create a shared device repository and write standardized code to flexibly generate the rig.json based on these local devices along with an additional file to select the device we wish to use. We don't need to do it now.

hagikent commented 2 months ago

My understanding is that this function is "Opto-out". So, ephys rigs can be simply excluded. For 24 boxes in the 446/447, I think it makes sense to automatize this as they are virtually identical except for Opto+/- and FIP+/-.

hanhou commented 2 months ago

I agree that we can merge this for now. From Alex's teams message, my understanding is that this PR is still a work in progress for rig.json but can move things forward. Let's think about how to deal with ephys rigs later (similar question applies to session.json).

alexpiet commented 2 months ago

The main reason is the devices and the code to generate the rig.json are very different across different rigs. If they were all hard-coded in the behavioral GUI, we would need to constantly update the code. For example, there are more fields that need to be added to rig.json (e.g. LED for optogenetics, IR light, patch cable, serial numbers of devices).

In 446/447/428 there are effectively three types of rigs: behavior, opto, FIP. For each of those rig types, the only thing we need to handle is the exactly serial number for each part. This can easily be included in the rig_specification.json. This hardware gets changed very infrequently, so we don't need to update the code often. We will only need to update the code when a new type of device is added (and maybe we can reorganize my code so we dont need to do that).

The ephys rigs can opt out of this process and generate their own rig.json files

Currently, everyone writes their own code to generate rig.json. I think we should have a unified repository to standardize this process. My idea is to create a shared device repository and write standardized code to flexibly generate the rig.json based on these local devices along with an additional file to select the device we wish to use. We don't need to do it now.

I think this is the start of that process. Its a work in progress. However, we need to start generating rig.jsons asap so we can move to the automatic data transfer process.

XX-Yin commented 2 months ago

I agree that we can merge this for now. From Alex's teams message, my understanding is that this PR is still a work in progress for rig.json but can move things forward. Let's think about how to deal with ephys rigs later (similar question applies to session.json).

Most of the information in the session json requires fields from the behavior GUI, but the rig.json is independent. My main concern is it will make the maintenance very difficult, and most likely we will have a centralized repo to create rig.json.

XX-Yin commented 2 months ago

The main reason is the devices and the code to generate the rig.json are very different across different rigs. If they were all hard-coded in the behavioral GUI, we would need to constantly update the code. For example, there are more fields that need to be added to rig.json (e.g. LED for optogenetics, IR light, patch cable, serial numbers of devices).

In 446/447/428 there are effectively three types of rigs: behavior, opto, FIP. For each of those rig types, the only thing we need to handle is the exactly serial number for each part. This can easily be included in the rig_specification.json. This hardware gets changed very infrequently, so we don't need to update the code often. We will only need to update the code when a new type of device is added (and maybe we can reorganize my code so we dont need to do that).

The ephys rigs can opt out of this process and generate their own rig.json files

Currently, everyone writes their own code to generate rig.json. I think we should have a unified repository to standardize this process. My idea is to create a shared device repository and write standardized code to flexibly generate the rig.json based on these local devices along with an additional file to select the device we wish to use. We don't need to do it now.

I think this is the start of that process. Its a work in progress. However, we need to start generating rig.jsons asap so we can move to the automatic data transfer process.

I understand the urgency to generate the rig.json. The rig.json can be generated independent (e.g. using the code I wrote). My suggestion is that the behavior GUI should still determine the update of rig.json (not necessarily through comparing the new and old rig.json), but the code to generate the rig.json should be kept independently to enable flexibility.

If we don't need to change the rig.json very frequently. Why not create a static rig.json? And we only need to update the calibration part every time when there is a new calibration.

alexpiet commented 2 months ago

@XX-Yin @hagikent @hanhou This is ready for review again. It now fills out all the details of the rig.json

alexpiet commented 2 months ago

@XX-Yin @hanhou @hagikent I tested this on 1C and 1D. I think we are ready to merge

XX-Yin commented 2 months ago

@XX-Yin @hanhou @hagikent I tested this on 1C and 1D. I think we are ready to merge

Okay, let's merge it and test the session metadata generation tomorrow.

alexpiet commented 2 months ago

@XX-Yin @hanhou @hagikent I tested this on 1C and 1D. I think we are ready to merge

Okay, let's merge it and test the session metadata generation tomorrow.

I made one quick change that I need to test in 447 to automatically load the FIP serial numbers. Then I will merge and test the session.json generation