Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inference verification plot #14

Merged
merged 14 commits into from
Apr 12, 2024
Merged

Conversation

clechartre
Copy link
Collaborator

@clechartre clechartre commented Apr 12, 2024

Purpose

Implements an extra-feature of the vis module to perform standalone plotting of the predictions. This allows for the verification of the inference output which outputs a Numpy array (see PR#13). Users can specify file path for saving and the channel (variable and level) to be plotted over the time range available in the inference output.

Code changes:

  • vis.py: Addition of the standalone function verify_inference, described in the purpose section of this description

  • weather_dataset.py: Addition of a split and specific datatype called "verification" which handles a numpy array instead of .zarr files, the current datatype supported by the model. The file is loaded as such, and, since it is backtransformed before storage, no specific transformation of the file is needed to pre-process it before plotting.

  • cli_plotting.py: Creation of a command line entry point to run the plotting on inference data as a standalone

  • README.md: adapted the readme according to this new functionality

Checklist

Before submitting this PR, please make sure:

  • You have followed the coding standards guidelines established at Code Review Checklist.
  • Docstrings and type hints are added to new and updated routines, as appropriate
  • All relevant documentation has been updated or added (e.g. README)

Review

For the review process follow the guidelines at Checklist

@clechartre clechartre requested a review from sadamov April 12, 2024 07:41
Copy link
Collaborator

@sadamov sadamov left a comment

Choose a reason for hiding this comment

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

Thanks! This will be very useful when plotting/evaluating various input formats in a standalone script. The two comments wrt 23 and 42 should be adressed before merge.
I think this PR would benefit from an additional script (slurm, CLI) for the user to interact with the standalone plotting. Some guidance on how to use it should then be added to the README. Would you have time to add such functionality?

neural_lam/vis.py Outdated Show resolved Hide resolved
neural_lam/vis.py Outdated Show resolved Hide resolved
neural_lam/weather_dataset.py Outdated Show resolved Hide resolved
@clechartre clechartre requested a review from sadamov April 12, 2024 12:16
sadamov added 5 commits April 12, 2024 15:38
- Arguments of function were in wrong order
- No need to return the last plot but free up memory
- slight changes to the save_path
Copy link
Collaborator

@sadamov sadamov 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! I made some smaller changes, as the code was not running on Balfrin initially. Please review my changes and if you agree squash commits and merge PR.
Thanks 🙏

README.md Outdated Show resolved Hide resolved
@clechartre clechartre merged commit e501435 into MeteoSwiss:main Apr 12, 2024
1 check passed
@clechartre clechartre deleted the verification branch April 12, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants