Skip to content

Conversation

@s6sebusc
Copy link

Description

Implements the option to read only the stored WeatherGen scores from JSON in the evaluation package, as requested in Issue #1202. To avoid duplication of code, the previous WeatherGenReader is split into a zarr reader and a json reader, which maybe wasn't the best idea.

Copy link
Collaborator

@iluise iluise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the code and I like the implementation. I will test it after it is updated to develop.

p.s. do you have ideas on how we could split the weathergen_reader in multiple files? it is getting quite long and it might be useful to slice it into more files


for run_id, run in runs.items():
reader = WeatherGenReader(run, run_id, private_paths)
reader = WeatherGenZarrReader(run, run_id, private_paths)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should test that this does not crash with mlflow or skip the runs that do not use zarrReaders

stream=list(self.eval_cfg.streams.keys())[0]
region=actual_eval_cfg.regions[0]
metric=actual_eval_cfg.metrics[0]
dummy = self.load_scores( stream, region, metric )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why dummy here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants