Conversation
ae7f36c to
41e6e51
Compare
41e6e51 to
3981c13
Compare
|
|
||
| Processing callback <callback/processing_callback.rst> | ||
| Optimizer callback <callback/optimizer_callback.rst> | ||
| Switch Scheduler <callback/switch_scheduler.rst> |
There was a problem hiding this comment.
I would keep the Scheduler callback, so that in the future, we can add other callbacks there
There was a problem hiding this comment.
This differs from #445, where we chose to name each file after the class it contains.
That said, your observation raises a valid point. My proposal is the following: within the callback directory, we create a dedicated scheduler subdirectory that, for now, will include only switch_scheduler.py, while leaving room for future additions. This prevents the file from becoming overly long as more scheduler-related callbacks are introduced.
The same structure would apply to optimizer_callback and processing_callback. In particular, the layout I have in mind is the following:
-- callback
-------- scheduler_callback
--------------- switch_scheduler.py
-------- optimizer_callback
--------------- switch_optimizer.py
--------- refinement
--------------- refinement_interface.py
--------------- r3_refinement.py
-------- processing_callback
--------------- metric_tracker.py
--------------- pina_progress_bar.py
--------------- normalizer_data_callback.py
Happy to hear your thoughts on this, @dario-coscia @FilippoOlivo @ndem0.
All these changes would be implemented in a dedicated PR.
There was a problem hiding this comment.
-- callback
-------- scheduler
--------------- switch_scheduler.py
-------- optimizer
--------------- switch_optimizer.py
--------- refinement
--------------- refinement_interface.py
--------------- r3_refinement.py
-------- processing
--------------- metric_tracker.py
--------------- pina_progress_bar.py
--------------- normalizer_data_callback.py
I prefer something like this (just remove callback on the subfolders)
There was a problem hiding this comment.
Very good! Maybe I would fuse scheduler and optimizer in optim directory (this is similar to from pina.optim import ...)
Description
This PR fixes #734
Checklist