Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

Bug: check for stretched grid breaks running with minimal graph config #203

Closed
dietervdb-meteo opened this issue Dec 14, 2024 · 11 comments · Fixed by #204
Closed

Bug: check for stretched grid breaks running with minimal graph config #203

dietervdb-meteo opened this issue Dec 14, 2024 · 11 comments · Fixed by #204
Labels
bug Something isn't working

Comments

@dietervdb-meteo
Copy link

dietervdb-meteo commented Dec 14, 2024

What happened?

The merging of PR199 introduced the line
if "lam_resolution" in getattr(config.graph.nodes.hidden, "node_builder", []): in forecaster.py
This will result in an error in case one sets up a config where the graph part is the minimal

graph:
  overwrite: False 
  data: data
  hidden: hidden

which is sufficient to load a graph previously stored on disk. Rather than trying to determine if the model is stretched grid or not from the graph part of the training config this should preferably be checked on the graph directly.

What are the steps to reproduce the bug?

Run training with a config for which the graph part is as above.

Version

current develop

Platform (OS and architecture)

Lumi [irrelevant]

Relevant log output

No response

Accompanying data

No response

Organisation

rmi

@dietervdb-meteo dietervdb-meteo added the bug Something isn't working label Dec 14, 2024
@dietervdb-meteo
Copy link
Author

@jswijnands , any simple fixes/ways around for this?

@dietervdb-meteo
Copy link
Author

As a temporary fix, the following minimal graph config still works [when running from a graph on disk]:

graph:
  overwrite: False #This to take graph from file
  data: data #This and next line needed by AnemoiModelEncProcDec in anemoi-models
  hidden: hidden 
  # below needed to circumvent the bug reported here [it doesn't do anything else in the code]
  nodes:
    hidden:
      node_builder:
        _target_: anemoi.graphs.nodes.LimitedAreaTriNodes # options: ZarrDatasetNodes, NPZFileNodes, TriNodes

@dietervdb-meteo
Copy link
Author

As mentioned in ecmwf/anemoi-core#30 , it would be elegant if the graph part of the training config when one wants to run from a graph on disk could be as minimal as:

graph:
   overwrite: False #or even something like create: False

@jswijnands
Copy link
Contributor

Hi Dieter, I see. We have indeed discussed various ways on how to identify whether a model is a stretched grid or not. Your minimal graph config use case was not envisioned then. The temporary fix you suggested indeed works.

@jswijnands
Copy link
Contributor

The if statement below may solve this problem, so you can still use a minimal script:

if "nodes" in getattr(config, "graph", []) and
    "hidden" in getattr(config.graph, "nodes", []) and
    "node_builder" in getattr(config.graph.nodes, "hidden", []) and
    "lam_resolution" in getattr(config.graph.nodes.hidden, "node_builder", []):

@jswijnands
Copy link
Contributor

Perhaps your preferred setup can also be used as one of the test scripts that is run when pushing new code changes. In that way, it gets flagged so it can be solved during development (also for future PRs).

@dietervdb-meteo
Copy link
Author

Concerning your proposed if statement above. That will indeed allow users that are not stretched grid and run graph from file to use a minimal config, but what about stretched grid users that would like to run with a minimal config and graph from file?

Maybe we can add some 'type' attribute to the graph, that can take values like 'stretched_grid', 'lam', 'icon' etc. That will allow other parts of the code to make decisions based on that. This type could be set through the config/or automatically detected through some if statements, but upon graph creation, rather than in another part of the code.

As this crosses between graph and training implementing something like that could be done once the repos are merged.

@JPXKQX , any comments/ideas?

@JPXKQX
Copy link
Member

JPXKQX commented Dec 16, 2024

This already exists. When you create a graph with anemoi-graphs each set of nodes has a node_type attribute whose value is the name of the class that created it:

>>> g["hidden"].node_type
'StretchedTriNodes'

@dietervdb-meteo
Copy link
Author

Great, thanks for pointing that out! @jswijnands , how about using the above to check if the model is stretched grid or not?

@jswijnands
Copy link
Contributor

Good idea, I'll look into this

@jswijnands jswijnands linked a pull request Dec 16, 2024 that will close this issue
@jswijnands
Copy link
Contributor

Hi Dieter, I've made this change and successfully tested it for stretched grid. See PR #204

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants