openclimatefix / graph_weather

PyTorch implementation of Ryan Keisler's 2022 "Forecasting Global Weather with Graph Neural Networks" paper (https://arxiv.org/abs/2202.07575)
MIT License
188 stars 47 forks source link

Assaf/fix resolution issue #75

Closed assafshouval closed 1 year ago

assafshouval commented 1 year ago

This fixes the issue mentioned in: https://github.com/openclimatefix/graph_weather/issues/46#issue-1438654907

Pull Request

Description

Fixes #46 The fix, is enforcing the dimension of the output of scatter_sum function, so it could be concatenate safely with the original nodes array. As of my understanding it is like padding with zeros to the output of scatter_sum.

How Has This Been Tested?

ran all the tests provided in test_model.py locally, and ran some more locally (I didn't believe the following tests should be added to the code base, so dumping it here):

 def forecaster(h3_res, angleDiff, feature_dim=78, aux_dim=24):
    lat_lons = []
    for lat in range(-90, 90, angleDiff):
        for lon in range(0, 360, angleDiff):
            lat_lons.append((lat, lon))
    model = GraphWeatherForecaster(
        lat_lons,
        resolution=h3_res,
        feature_dim=feature_dim,
        aux_dim=aux_dim,
        node_dim = 2,
        edge_dim = 3,
        num_blocks = 9,
        hidden_dim_processor_node = 4,
        hidden_dim_processor_edge = 5,
        hidden_layers_processor_node = 2,
        hidden_layers_processor_edge = 2,
        hidden_dim_decoder = 1,
        hidden_layers_decoder = 2,

    )
    # Add in auxiliary features ( feature_dim: int = (78,) aux_dim: int = 24,)
    features = torch.randn((1, len(lat_lons), feature_dim + aux_dim))

    out = model(features)
    assert not torch.isnan(out).any()
    assert not torch.isnan(out).any()

def test_forecaster():
    forecaster(3, angleDiff=5, feature_dim=3, aux_dim=3)
    forecaster(2, 5, 78, 24)
    forecaster(2, angleDiff=10, feature_dim=78, aux_dim=24)
    forecaster(2, angleDiff=30, feature_dim=78, aux_dim=24)
    forecaster(2, angleDiff=90, feature_dim=78, aux_dim=24)
    forecaster(3, angleDiff=90, feature_dim=3, aux_dim=3)
    forecaster(2, 5)
    forecaster(1, 1)
    forecaster(1, 5)
    forecaster(2, angleDiff=10)
    forecaster(3, angleDiff=5)

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

Checklist:

assafshouval commented 1 year ago

@jacobbieker do you know why it is failing? all the files seems irrelevant for my PR

jacobbieker commented 1 year ago

Yeah, we've updated the linting for all the OCF repos, and the stricter rules are causing the failures here, which we haven't fixed yet. I'll need to go in and update this repo to match that soon. But am happy to merge this if you are finished with it?

assafshouval commented 1 year ago

Yes, I'm finished with it

בתאריך יום ב׳, 11 בספט׳ 2023, 12:01, מאת Jacob Bieker ‏< @.***>:

Yeah, we've updated the linting for all the OCF repos, and the stricter rules are causing the failures here, which we haven't fixed yet. I'll need to go in and update this repo to match that soon. But am happy to merge this if you are finished with it?

— Reply to this email directly, view it on GitHub https://github.com/openclimatefix/graph_weather/pull/75#issuecomment-1713469446, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHSJL7W5CF4FLWCK2HQF43XZ3HP7ANCNFSM6AAAAAA4SGPK5E . You are receiving this because you authored the thread.Message ID: @.***>