Skip to content

Conversation

glitch401
Copy link

Pull Request

Description

This PR addresses a core architectural limitation by refactoring the GraphWeatherForecaster to support configurable input and output features, removing the previously hardcoded dimensions. This change significantly improves the repository's flexibility and makes it easier to experiment with new models and feature sets.

This was done by:

  1. Introducing a FeatureManager class (graph_weather/models/features.py) to serve as a single source of truth for feature definitions.
  2. Externalizing feature sets into human-readable YAML files within a new graph_weather/configs/ directory.
  3. Refactoring the GraphWeatherForecaster and its Config to accept dynamic input_features and output_features arguments, decoupling the model architecture from the data schema.

This work provides a crucial foundation for implementing new models (like CaFA in #105) that may use different input variables. It also aligns with the goals of issue #126 by introducing a more structured configuration approach.

To ensure full backward compatibility, a features_default.yaml file is included that perfectly replicates the original hardcoded 78+24 feature set.

Fixes #

How Has This Been Tested?

The refactored GraphWeatherForecaster and FeatureManager were tested using test scripts and interactively in a Jupyter notebook to validate:

  • Correct parsing of the default 102-feature configuration.

  • Correct parsing of a minimal, custom 1-feature configuration.

  • Successful model instantiation and forward passes with both fake data and a real single-level ERA5 dataset, proving the end-to-end flexibility of the new system.

  • Yes

If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?

  • Yes
    (N/A - This change refactors the model's interface but does not alter the data processing logic itself.)

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

Copy link
Member

@jacobbieker jacobbieker left a comment

Choose a reason for hiding this comment

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

Jsut a note on this one, I don't think the model features needs to be refactored like this. Its not hardcoded currently, and I think this adds more complexity than it helps personally. The current values can be overwritten with a config dictionary, so already doing what this is trying to do.

@glitch401
Copy link
Author

Your feedback makes sense. Would you suggest that the input features to dynamically adapt according to the input features?

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