AllenNeuralDynamics / aind-behavior-curriculum

Starter repository for behavior base primitives.
https://aind-behavior-curriculum.readthedocs.io
MIT License
1 stars 0 forks source link

Consider adding `stage_name` descriptor to `Task` object #33

Closed bruno-f-cruz closed 3 months ago

bruno-f-cruz commented 4 months ago

Is your feature request related to a problem? Please describe. From the point of view of the Task schema, no field is available to keep track of stage or experiment type. This would be interesting as the same Task could be used, via different parameters, to instantiate different experiments (or stages).

As I was thinking about this, it felt like violating a bit the separation of concerns principle but I think it may be warranted for a few reasons:

  1. If a user wants to buy into the Task specification but does not want to use the full Curriculum (Curriculum actually deserializes the stage information but only with the full curriculum, not the suggestion)
  2. For instances o Task kept in a remote database (e.g. slims) it would be nice to have a human readable field that indicated with stage or experiment it is being suggested by the Task json instance.

Describe the solution you'd like

Add stage_name to Task with the following signature:

`stage_name`: Optional[str] = Field(default=None)

The following should be added:

Describe alternatives you've considered

Leave it up to the user to add this field to TaskParameters. In which case it would not be the concern of the curriculum. But it would also be much harder to generate a common API that uses this field

I am a bit afraid of the possible duplication of metadata between Stage.name and this suggested property. If that is a problem, we may just want to change the name of this property for something else to prevent confusion. Perhaps task_descriptor?

Additional context This field should only be used for generating human-readable metadata, I would not rely on it for any functional implication in the curriculum. Instead we should rely on Stage.name

dyf commented 3 months ago

Seems fine to me. Although I think you are saying that Take.stage_name should not be optional, so I don't think you need to add any special validation.

bruno-f-cruz commented 3 months ago

To clarify, the validation step should be done at the level of the Stage class, not Task. name in Stage is required: https://github.com/AllenNeuralDynamics/aind-behavior-curriculum/blob/bb28161608b4544e5f71e23c2190a6682f17dcaf/src/aind_behavior_curriculum/curriculum.py#L437