Improve stdataset - #78
Conversation
There was a problem hiding this comment.
Hi @SarahAlidoost , thanks for the reorganization! I have looked into the changes, and managed to run several epochs of the hourly notebook without OOM error.
The performance on my local for the hourly notebooks is ~7mins per epoch, with 4 threads computing and 2 workers reading. This is comparible with the timing profile in your notebook.
I only made one small comment about whether we should always loop in time. It is not critical. If your 1-year experiment on Levante works well, feel free to merge!
And PS. there is one linting issue raised by Ruff, I guess you also saw it.
|
|
||
| # Track unscaled loss for logging | ||
| epoch_loss += loss.detach() | ||
| for year in years: |
There was a problem hiding this comment.
Do we always want to loop by year, or for some circumstance we want to allow multiple years?
There was a problem hiding this comment.
We are training at a monthly aggregation anyway, so it doesn't really matter I think. The loop basically goes over all months, grouped by year (so allows for multiple years) within one epoch.
| store_model: bool = True | ||
|
|
||
|
|
||
| def _move_batch_to_device(batch: dict, device: str): |
There was a problem hiding this comment.
This is a duplication of the class methos also used in predict.py. Given the very nice work you've done in cleaning and structuring one could consider moving it to utils and reusing in both modules. However, this is a most a suggestion - feel free to ignore
meiertgrootes
left a comment
There was a problem hiding this comment.
Very nice and clean organization! One very minor comment and a response to @rogerkuou 's . Works locally. if/when levante test is complete feel free to merge!
closes #75
In this PR:
STDataset class has an option to keep data lazy in--> data of training should not be lazyinitand instead load it ingetitem: save memory but slowerI'm still testing the
example_hourlynotebook on HPC. but this PR can be reviewed.TODO: