Sorcha/dev/zarr3 compaction #1450
Conversation
github-actions
bot
added
infra
|
Latest version can read and write with the zipstore format but needs tidying up (produces lots of print statements to narrow down the issues when trying to run the evaluation from a .yml file) |
|
|
Integration test fails, need to investigate this further! |
What is the performance impact for running evaluation with using zipstore? |
tjhunter
left a comment
tjhunter
left a comment
There was a problem hiding this comment.
Some small style comments. Otherwise, it looks ready to try out on a larger scale.
When testing into the logic, I moved the creation of the zarr store straight into inference. Otherwise, I am personally confused when we write during inference and when we just calculate validation losses.
@grassesi : do you know why we don't write data when calling validation during training?
| """Convert raw dask arrays into chunked dask-aware xarray dataset.""" | ||
| chunks = (chunk_nsamples, *self.data.shape[1:]) | ||
|
|
||
| chunks = (chunk_nsamples, *(max(x // 4, 1) for x in self.data.shape[1:])) |
There was a problem hiding this comment.
where is this formula coming from? Someone else should be able to update it later.
There was a problem hiding this comment.
extracted out into a scale factor parameter now for easier updating
| self.data_root: zarr.Group | None = None | ||
| self._store: LocalStore | None = None | ||
| # determine type using extension | ||
| ext = self._store_path.suffix[1:] |
There was a problem hiding this comment.
you are repeating yourself
| if self.type == "zip": | ||
| if self.create: | ||
| _logger.info("Creating zipstore") | ||
| self._store = ZipStore(self._store_path, mode="a") |
There was a problem hiding this comment.
append is more permissive, but there is a cost in opening and closing the store. I would rather have the store opened for the full duration of the write rather than for each chunk.
There was a problem hiding this comment.
mode = "w" works with the new develop code, before I had some issues with overwriting so was using append - thanks for fixing this :)
We don't write during validation since it's slows down the validation, generates additional files/storage, and is rarely used. There's a config parameter that controls this: WeatherGenerator/config/default_config.yml Line 337 in 1776b0a |
…rcha/dev/zarr3_compaction
github-actions
bot
added
the
initiative
Hi Christian, sorry I missed this before the holidays - the latest tests are all in the hedge doc: https://gitlab.jsc.fz-juelich.de/hedgedoc/3X10vtdVQumQZy4qpBiHWg - there seems to be an small imporevemnt when using the ZipStore to evaluate |
Description
Draft PR to showcase how to enable sharding (experimentation underway to optimise performance)
Issue Number
DRAFT
Closes #1384
Checklist before asking for review
./scripts/actions.sh lint./scripts/actions.sh unit-test./scripts/actions.sh integration-testlaunch-slurm.py --time 60