skillful_nowcasting icon indicating copy to clipboard operation
skillful_nowcasting copied to clipboard

Refactor serialization via Huggingface Hub

Open agijsberts opened this issue 1 year ago • 0 comments

Pull Request

Description

This PR follows the discussion in https://github.com/openclimatefix/skillful_nowcasting/issues/68 and makes DGMR compatible with recent updates in huggingface_hub.

Specifically, it removes the explicit config argument from constructors. This simplifies the constructors, but may cause some backwards compatibility problems if people rely on passing this parameter. However, it should be easy to fix this by replacing DGMR(config=myconfig) with DGMR(**myconfig).

Secondly, this PR also contains a separate commit that replaces the custom NowcastingModelHubMixin with huggingface_hub.PyTorchModelHubMixin. This does not have any real advantages, aside from trimming redundant code. It should be noted that altough PyTorchModelHubMixin can read both the PyTorch's .bin and safetensors formats, it can only write the latter. So any potential future updates to the model would migrate the format automatically to safetensors.

How Has This Been Tested?

Tests have been added in tests/test_model.py that verify that models can be serialized and unserialized without any changes to the parameters or hyperparameters. These fixes passes all tests.

Additionally, I have manually verified that predictions on random data are identical to those produced by the previous version.

Checklist:

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

(I've attempted to follow OCF's coding style, but running black causes many changes in existing code that is not touched by this PR)

agijsberts avatar Jul 31 '24 14:07 agijsberts