Skip to content

feat: Add version validation if model fails to load #194

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

Merged
merged 9 commits into from
Apr 24, 2025

Conversation

HCookie
Copy link
Member

@HCookie HCookie commented Mar 25, 2025

Description

Add environment context information if the model fails to load.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Code Compatibility

  • I have performed a self-review of my code

Code Performance and Testing

  • I have added tests that prove my fix is effective or that my feature works
  • I ran the complete Pytest test suite locally, and they pass

Dependencies

  • I have ensured that the code is still pip-installable after the changes and runs
  • I have tested that new dependencies themselves are pip-installable.

Documentation

  • My code follows the style guidelines of this project
  • I have updated the documentation and docstrings to reflect the changes
  • I have added comments to my code, particularly in hard-to-understand areas

Additional Notes

@github-project-automation github-project-automation bot moved this to Now In Progress in Anemoi-dev Mar 25, 2025
@github-actions github-actions bot added the enhancement New feature or request label Mar 25, 2025
@HCookie HCookie self-assigned this Mar 25, 2025
@HCookie HCookie requested a review from aaron-hopkinson March 25, 2025 11:26
@HCookie HCookie moved this from Now In Progress to Under Review in Anemoi-dev Mar 25, 2025
@HCookie HCookie force-pushed the feat/add_version_info_when_model_fails branch from 1336a7d to 9a70088 Compare March 25, 2025 13:27
Copy link
Contributor

@aaron-hopkinson aaron-hopkinson left a comment

Choose a reason for hiding this comment

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

Some minor comments - may be out of the scope of this PR, but I feel like we might want to clean this code at some point.

Also if we're adding a new feature, we should probably add a test - just something simple like mock/patching the call to torch.load, forcing it to raise an exception, and ensuring that the validate_environment is called, is probably sufficient to check that the high-level behaviour is doing what we expect (rather than the details of what's returned etc).

Copy link
Member

@gmertes gmertes left a comment

Choose a reason for hiding this comment

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

LGTM. I share Aaron's concerns but we also want this function to be flexible. We could look at improving the documentation a bit, but it's good enough imo.

@mchantry mchantry moved this from Under Review to ATS in Anemoi-dev Apr 16, 2025
@HCookie HCookie merged commit bfd890f into main Apr 24, 2025
68 checks passed
@HCookie HCookie deleted the feat/add_version_info_when_model_fails branch April 24, 2025 10:50
@github-project-automation github-project-automation bot moved this from ATS to Done in Anemoi-dev Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants