Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Conversation

@JackKelly
Copy link
Contributor

@JackKelly JackKelly commented Nov 16, 2021

Pull Request

PLEASE NOTE THAT THERE ARE ONLY TWO SMALL CHANGES IN THIS PR IN nwp_data_source.py: HERE AND HERE

The reason GitHub says 7 files have changed is because I merged a bunch of other bug fixes into this branch. Specifically:

Good news: prepare_ml_data.py now creates NWP batches! And fairly fast, too (about 1 per second).

Fixes #410

How Has This Been Tested?

  • No
  • Yes, unit tests pass
  • prepare_ml_data.py runs successfully

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@JackKelly JackKelly added the bug Something isn't working label Nov 16, 2021
@JackKelly JackKelly self-assigned this Nov 16, 2021
@JackKelly JackKelly changed the base branch from main to jack/temporary-merge-of-log-to-files-and-manger-sun November 16, 2021 21:19
@JackKelly JackKelly changed the base branch from jack/temporary-merge-of-log-to-files-and-manger-sun to main November 16, 2021 21:20
@JackKelly JackKelly marked this pull request as ready for review November 16, 2021 21:22
zarr_path, engine="zarr", consolidated=consolidated, mode="r", chunks=None
)

ukv = nwp["UKV"]
Copy link
Contributor

Choose a reason for hiding this comment

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

is the reversing needed for other channels?

Copy link
Contributor

Choose a reason for hiding this comment

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

I may have miss understood here - UKV might not be a channel. Perhaps add a comment why "UKV" is selected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, good point! UKV is the name of the (one and only) DataArray in the Zarr Dataset. You're right, UKV isn't a channel. UKV is the name of the DataArray which holds all the channels (the channels are represented as a dimension in the DataArray). I'll add a comment! Thanks!

Copy link
Contributor

@peterdudfield peterdudfield left a comment

Choose a reason for hiding this comment

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

Looks good.

  • just a small comment
  • looks like a few conflicts, but im sure you can merge main into this first

@JackKelly JackKelly merged commit 9720f47 into main Nov 16, 2021
@JackKelly JackKelly deleted the jack/bug/nwp-example-is-wrong-shape branch November 16, 2021 21:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NWPDataSource RuntimeError: Example is wrong shape!

3 participants