Reorganize `Trainer` class
Although everything works as intended now, and is reasonable logical, it would be nice to adhere to the logic of pytorch lightning (see: here)
Reorganize which arguments are passed to Trainer directly (in the init) and which are passed to train method within Trainer.
Trainer class should contain:
- epochs, batch size, shuffle,
- metrics exporter,
- early stop, save option, etc
Trainer.train() method should have inputs:
- choice of neuralnet/model
- different datasets
- i.e., either train/validation/test datasets or val_size/test_size
- class_weights
- consider renaming the method
fitto align with pytorch lightning
Some (many?) of the internal methods of Trainer would now have to be called at a different point:
_put_model_to_device()- ...
related issues: #153, #156, #287
This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.
@gcroci2, are we closing this issue?
@gcroci2, are we closing this issue?
I think this may be a very nice enhancement that will take very little time. I'd leave it open for now :)
I think it's not that straightforward. I think was working on this a while ago and got completely stuck and ended up having to ditch the entire branch, because it became way more convoluted than that it clarified anything and still didn't really work the way we imagined. Or am I now confusing this with a similar issue?
I should've commented on it back then, which I apparently failed to do.
I think it's not that straightforward. I think was working on this a while ago and got completely stuck and ended up having to ditch the entire branch, because it became way more convoluted than that it clarified anything and still didn't really work the way we imagined. Or am I now confusing this with a similar issue?
I should've commented on it back then, which I apparently failed to do.
Wasn't it in the HDF5Dataset (now GraphDataset) class? I think you were trying to move something related to the get method to the Trainer, and that was a mess indeed. But I am also not completely sure now, so maybe we can give a fast look again at this at a certain point, and if we realize is too complex we'll close it then?
I think it's not that straightforward. I think was working on this a while ago and got completely stuck and ended up having to ditch the entire branch, because it became way more convoluted than that it clarified anything and still didn't really work the way we imagined. Or am I now confusing this with a similar issue? I should've commented on it back then, which I apparently failed to do.
Wasn't it in the HDF5Dataset (now GraphDataset) class? I think you were trying to move something related to the get method to the Trainer, and that was a mess indeed. But I am also not completely sure now, so maybe we can give a fast look again at this at a certain point, and if we realize is too complex we'll close it then?
I think you're right, actually