search

AllenNeuralDynamics / aind-behavior-curriculum

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

Why do we need Stage and Task objects? #31

Open jeromelecoq opened 4 months ago

jeromelecoq commented 4 months ago

https://github.com/AllenNeuralDynamics/aind-behavior-curriculum/blob/bb28161608b4544e5f71e23c2190a6682f17dcaf/examples/example_project/curriculum.py#L171

It looks like the parameters are directly pass to the a stage object. Could we just have a Stage object? What is the usefulness of these 2 layers?

bruno-f-cruz commented 4 months ago

Not sure I completely follow the question. A Task is a base class that users should inherit from to construct their task objects and respective parameters. It defines a schema to (de)serialize the task object. Stage is a container for a task INSTANCE (not schema) and logic regarding policies and policy transitions. There is a reason why Stage is in the curriculum module and not the task one.

Another question could be: Stage inherit from Task? We actually started with that and quickly concluded that it would be super annoying since users would have to create new Stage classes for each Task they might want to run, without being able to re-use the Task classes. That's why we opted for the functional programming syntax and simply look at Stage as a container of Task instance + extra logic.

This pattern also affords a bit of separation of concerns. The user can start by using Task objects to define their task and only later adopt the full curriculum. We can discuss why this is important, but part of my pet peeve to have modular systems and allow users to use common tools, like databases (looking at you SLIMS) with required fields (aka schemas), to store their task settings without having to buy into the full automatic training pipeline.

If you have a different implementation in mind, could you maybe make a small draft PR so I could study what it looks like?

jeromelecoq commented 4 months ago

I am still confused and a class architecture could help there. Why not have the Task functionality at the same level as Stage? You could have function to deserialize at any level?

I could make a PR but I still don't understand your current architecture so it would be difficult for me to provide alternate solutions.