Conversation
| data_streams=[ | ||
| Json( | ||
| name="PreviousMetrics", | ||
| reader_params=Json.make_params( | ||
| reader_params=PydanticModel.make_params( | ||
| model=Metrics, |
There was a problem hiding this comment.
This is fine (and even a good practice.) However, keep in mind this requires you not to make breaking changes to the Metrics model. IF you break it, the contract will not be backwards compatible anymore.
| Optional[float]: Total volume of water consumed in milliliters, or None if unavailable. | ||
| """ | ||
|
|
||
| trial_outcomes = dataset(session_path)["Behavior"]["SoftwareEvents"]["TrialOutcome"].load().data["data"] |
There was a problem hiding this comment.
You can validate this with the TrialOutcome pydantic class to make your life easier
| from aind_behavior_dynamic_foraging.task_logic import TrialGeneratorSpec | ||
| logging.basicConfig(stream=sys.stdout, level=logging.DEBUG) | ||
|
|
||
| from aind_behavior_dynamic_foraging.task_logic import TrialGeneratorSpec # noqa |
There was a problem hiding this comment.
If I'm remembering correctly, it was because I was failing the linting tests since the imports weren't grouped at the top of the file and I had to set up logging before importing the trail generators
| trainer_state = dataset["Behavior"]["TrainerState"].data | ||
| performance_metrics = PerformanceMetrics(output_parameters=metrics.model_dump()) | ||
|
|
||
| stimulus_epoch = StimulusEpoch( |
There was a problem hiding this comment.
This seems rather arbitrary. What about quick retract, reward delivery, secondary reinforcer (?). I will leave it up to the team to decide what is relevant and what is not
There was a problem hiding this comment.
@alexpiet any thoughts here for what should be included in the stimulus epoch? I was basing this on what I saw in the current dynamic-foraging-task and only saw go cue, optogenetics, random reward, and optical tagging.
There was a problem hiding this comment.
If quick retract is in use, it should be noted somewhere in the metadata. Same for secondary reinforcers. I'm not sure what you mean @bruno-f-cruz by reward delivery.
I don't know any details about optogenetics, random reward, or optical tagging. I'm fairly certain that optical tagging should not be part of this refactor.
There was a problem hiding this comment.
I probably just don't understand what the stimulus_epoch is meant to represent.
https://aind-data-schema.readthedocs.io/en/latest/aind_data_schema_models/stimulus_modality.html#stimulusmodality
You have Visual in the same category as Free moving. This confuses me, as I am not sure if we are supposed to encode DynamicForaging = Stimulus, or instead encode all the materialized effects that the task logic has on the world. If the latter, I do think that "reward" is one of them (at the limit, the sensory experience that results from the water coming out).
Anyway, I think you should just encode whatever makes sense to you, and you will need to query the docdb.
There was a problem hiding this comment.
StimulusModality should just be auditory, and if present optogenetics. Free moving is a stimulus because if the mouse is freely moving then it can self-stimulate.
| semver += f"-{v.releaselevel}.{v.serial}" | ||
| return Code( | ||
| url=repository.remote().url, | ||
| name="aind-behavior-vr-foraging", |
There was a problem hiding this comment.
This File does not need to be created at the rig afaik. Remove it unless necessary.
There was a problem hiding this comment.
You should talk to people from sci comp. As far as I understand this is considered an anti-pattern since they want you to keep this static somewhere.
There was a problem hiding this comment.
Reached out to Arielle and she said it's preferable to dynamically generate at rig
| dependencies = [ | ||
| "numpy>=2.4.2", | ||
| "pydantic-settings", | ||
| "aind-behavior-dynamic-foraging==0.0.2rc24", |
There was a problem hiding this comment.
This should just be able to reference the current version in the repo I feel (?)
|
@arielleleon @saskiad this is the pr mapping the dynamic foraging output into an acquisition and instrument model. |
Adds metadata mapping for acquisition, instrument, and data description as well as a cli. Based of vr foraging metadata mappers. Also some fixes for coupled trial generator. There are some things in data description like funding and investigators that need to be worked out