-
Notifications
You must be signed in to change notification settings - Fork 19
Add barseq acquisition #1690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1642-create-instrumentjson-generator-for-barseq
Are you sure you want to change the base?
Add barseq acquisition #1690
Conversation
| "dx.doi.org/10.17504/protocols.io.n2bvj82q5gk5/v1", | ||
| "dx.doi.org/10.17504/protocols.io.81wgbp4j3vpk/v2" | ||
| ], | ||
| "ethics_review_id": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no iacuc for in vitro experiments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in c0e949b
| "PLACEHOLDER_EXPERIMENTER_1", | ||
| "PLACEHOLDER_EXPERIMENTER_2" | ||
| ], | ||
| "protocol_id": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why two protocols for the same thing? And why not our own protocol: https://www.protocols.io/view/barseq-2-5-kqdg3ke9qv25/v1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those two protocols came from the methods doc that Polina shared. I didn't know about this one. Will add instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 4ab930b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe confirm first. This is the protocol Yoh uses, so there might be a chance it's not the same?
But, the two protocols you had at first, one was an update of the other, so we should only use the one that's being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polina confirmed that this new protocol is the correct one.
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "Gene_G", | ||
| "intended_measurement": "Gene sequencing - DNA base G", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not an appropriate intended measurement. DNA Base G is not a gene.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See change here:
4f6f2c9
| "channels": [ | ||
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "Gene_G", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as next comment: G is not a gene.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See change here: 4f6f2c9
examples/2026.01.16_barseq_acquisition/barseq_780346_acquisition.json
Outdated
Show resolved
Hide resolved
| }, | ||
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "Gene_DAPI", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. Dapi is not a gene
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See change here: 4f6f2c9
| } | ||
| ], | ||
| "coordinate_system": null, | ||
| "images": [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be populated. This is what is being imaged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 3824c9e
| ], | ||
| "connections": [] | ||
| }, | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure there should be multiple data streams here. (and I'm not sure there shouldn't, but thinking out loud here). Each round of imaging creates a new image, so I would argue each round of imaging is a separate acquisition. We could decide each round is a separate data stream - that could work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an opinion here. Just need some guidance on how to implement whatever is most sensible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If each round creates a distinct raw asset, then each round is a separate acquisition. We need to know how that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure what to do with that comment. Can you help me turn that into a concrete change to make to the acquisition generator and/or a question to pose to Polina or Xiaoyin?
Are you saying that we should actually be creating three different acquistion.json files?
| ], | ||
| "instrument_id": "Dogwood", | ||
| "acquisition_type": "BarcodeSequencing", | ||
| "notes": "BARseq acquisition of Locus Coeruleus for noradrenergic neuron projection mapping. Specimen: 51 coronal sections (20μm) spanning CCF plates 99-112. Subject 780346 received Sindbis HZ120 virus injection 22-28h pre-harvest. BARseq experiment performed using automation template mounted on microscope. Automated microfluidics setup for reagent delivery.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm generally not excited about having notes that contain the information we're trying to encode in the metadata because it enables people to develop bad habits of parsing notes instead of using the metadata properly. Likewise below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes are less verbose now. I can remove them entirely if you'd prefer.
saskiad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments
| ], | ||
| "code": null, | ||
| "notes": "Gene barcode sequencing (7 cycles) using sequential base incorporation imaging", | ||
| "active_devices": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to list things like objective or spinning disk or filters. The things that have configs or are creating data. So cameras, lasers,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For filters, there's a field for "emission_filters" in the channel configs. See: https://aind-data-schema.readthedocs.io/en/latest/components/configs.html#channel
And they have a "device_name" field. See https://aind-data-schema.readthedocs.io/en/latest/components/configs.html#deviceconfig
As it stands, the device_names in the emission_filters align with the names in the active_devices. Is this not necessary? Should I retain the names in "emission_filters" and drop them from active_devices, or drop them in both places?
| } | ||
| ], | ||
| "code": null, | ||
| "notes": "Gene barcode sequencing (7 cycles) using sequential base incorporation imaging", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note feels weird to me - I would think saying something like cycle 1 of 7 makes sense, but this isn't the entire 7 cycles, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "configurations": [ | ||
| { | ||
| "object_type": "Imaging config", | ||
| "device_name": "Ti2-E__0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what device this is - the main microscope? Is this the correct device to pin to? We can use the instrument itself (correct me if I'm wrong @dbirman)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the imaging config here should point to the full instrument_id in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the field name should say "device_name": "Dogwood"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See e80306e
| "channels": [ | ||
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "GeneSeq_G", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a preference thing, but I would name the channel around the color if that's possible and leave the meaning to the intended measurement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't yet have the mapping for base to wavelength. And even when we do, I'm afraid there might be some subjectivity in how color names are mapped to wavelength.
I think we have 3 options:
- Leave as is (channel identifies base being imaged)
- Name channels with wavelength (precise, but maybe not intuitive)
- Name channels with color. If we do this, I'll need guidance on how to assign color names.
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "GeneSeq_G", | ||
| "intended_measurement": "Fluorescent signal from sequencing reaction indicating guanine incorporation", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be better to make this less verbose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "Guanine" is sufficient, personally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See b29bf11
| }, | ||
| { | ||
| "object_type": "Channel", | ||
| "channel_name": "GeneSeq_DAPI", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove "GeneSeq" since DAPI is not part of the sequencing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to just "DAPI". See 58759e6
saskiad
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More comments - great job on the imaging config. I think the big question is about whether each round is a distinct raw asset or not.
…the instrument_id)
How do we resolve that question? Does it come down to how data is actually stored? Do we need to reach out to Xiaoyin or someone else? |


Work in progress. Lots of ancillary files that we won't actually want in the repo. Remove before merging.
Closes: https://github.com/AllenNeuralDynamics/aind-scientific-computing/issues/569