Skip to content

fix(graphs): throw error instead of raising warning when graph exists. #379

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 25, 2025
Merged

fix(graphs): throw error instead of raising warning when graph exists. #379

merged 4 commits into from
Jun 25, 2025

Conversation

@matschreiner
Copy link
Contributor

@matschreiner matschreiner commented Jun 24, 2025

Description

The warning currently thrown by anemoi-graphs create when a graph already exists is too subtle. If you don’t scroll up in a successful-looking output, you will miss that the graph creation failed. Here’s an example output from a failed graph creation from my terminal:

Screenshot from 2025-06-24 08-34-22

I began working on a new PR because I thought there was a bug elsewhere, because I didn’t notice the warning.

To make failure more noisy, I’ve changed the behavior to raise an explicit error instead of a warning:

Screenshot from 2025-06-24 08-58-28

As a contributor to the Anemoi framework, please ensure that your changes include unit tests, updates to any affected dependencies and documentation, and have been tested in a parallel setting (i.e., with multiple GPUs). As a reviewer, you are also responsible for verifying these aspects and requesting changes if they are not adequately addressed. For guidelines about those please refer to https://anemoi.readthedocs.io/en/latest/

@matschreiner
Copy link
Contributor Author

matschreiner commented Jun 24, 2025

@JPXKQX

@matschreiner matschreiner changed the title fix (graphs): throw error instead of raising warning when graph exists. fix(graphs): throw error instead of raising warning when graph exists. Jun 24, 2025
mchantry
mchantry approved these changes Jun 24, 2025
View reviewed changes
Copy link
Member

@mchantry mchantry left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.

@matschreiner
Copy link
Contributor Author

matschreiner commented Jun 24, 2025

@mchantry I'm not sure how to fix the failing check with labels, I don't have permission to do anything related to labels

@github-project-automation github-project-automation bot moved this to Now In Progress in Anemoi-dev Jun 25, 2025
@mchantry mchantry merged commit 6ec6c18 into ecmwf:main Jun 25, 2025
17 checks passed
@github-project-automation github-project-automation bot moved this from Now In Progress to Done in Anemoi-dev Jun 25, 2025
@DeployDuck DeployDuck mentioned this pull request Jun 25, 2025
Merged
@JPXKQX JPXKQX mentioned this pull request Jul 8, 2025
Merged
JPXKQX added a commit that referenced this pull request Jul 8, 2025
@JPXKQX @pre-commit-ci
fix(graphs): revert PR #379 (#409)
d51219f 
## Description
<!-- What issue or task does this change relate to? -->

Revert PR #379 .

Thanks to @havardhhaugen for spotting this.

## What problem does this change solve?
<!-- Describe if it's a bugfix, new feature, doc update, or breaking
change -->

Raising an error during graph creation will break multi-GPU training.
Currently, one limitation of `anemoi-graphs` is that it does not support
distributed creation. For example, during multi-GPU training with
on-the-fly graph creation, the same graph is created on all GPUs. In
this case, raising an error will cause the code to fail on some of the
GPUs.

## What issue or task does this change relate to?
<!-- link to Issue Number -->

##  Additional notes ##
<!-- Include any additional information, caveats, or considerations that
the reviewer should be aware of. -->

***As a contributor to the Anemoi framework, please ensure that your
changes include unit tests, updates to any affected dependencies and
documentation, and have been tested in a parallel setting (i.e., with
multiple GPUs). As a reviewer, you are also responsible for verifying
these aspects and requesting changes if they are not adequately
addressed. For guidelines about those please refer to
https://anemoi.readthedocs.io/en/latest/***

By opening this pull request, I affirm that all authors agree to the
[Contributor License
Agreement.](https://github.com/ecmwf/codex/blob/main/Legal/contributor_license_agreement.md)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
OpheliaMiralles pushed a commit that referenced this pull request Jul 9, 2025
@matschreiner @mchantry @OpheliaMiralles
fix(graphs): throw error instead of raising warning when graph exists. (
c7051c4 
#379)

## Description

The warning currently thrown by `anemoi-graphs create` when a graph
already exists is too subtle. If you don’t scroll up in a
successful-looking output, you will miss that the graph creation failed.
Here’s an **example output from a failed graph creation** from my
terminal:

![Screenshot from 2025-06-24
08-34-22](https://github.com/user-attachments/assets/0bb36d6e-d488-4689-b527-a73ba1a46ddb)

I began working on a new PR because I thought there was a bug elsewhere,
because I didn’t notice the warning.

To make failure more noisy, I’ve changed the behavior to raise an
explicit error instead of a warning:

![Screenshot from 2025-06-24
08-58-28](https://github.com/user-attachments/assets/96ab03fa-161a-4bfe-bc9f-75976b690778)


 ##
<!-- Include any additional information, caveats, or considerations that
the reviewer should be aware of. -->

***As a contributor to the Anemoi framework, please ensure that your
changes include unit tests, updates to any affected dependencies and
documentation, and have been tested in a parallel setting (i.e., with
multiple GPUs). As a reviewer, you are also responsible for verifying
these aspects and requesting changes if they are not adequately
addressed. For guidelines about those please refer to
https://anemoi.readthedocs.io/en/latest/***

Co-authored-by: Matthew Chantry <[email protected]>
OpheliaMiralles pushed a commit that referenced this pull request Jul 9, 2025
@JPXKQX @pre-commit-ci @OpheliaMiralles
fix(graphs): revert PR #379 (#409)
f4a3936 
## Description
<!-- What issue or task does this change relate to? -->

Revert PR #379 .

Thanks to @havardhhaugen for spotting this.

## What problem does this change solve?
<!-- Describe if it's a bugfix, new feature, doc update, or breaking
change -->

Raising an error during graph creation will break multi-GPU training.
Currently, one limitation of `anemoi-graphs` is that it does not support
distributed creation. For example, during multi-GPU training with
on-the-fly graph creation, the same graph is created on all GPUs. In
this case, raising an error will cause the code to fail on some of the
GPUs.

## What issue or task does this change relate to?
<!-- link to Issue Number -->

##  Additional notes ##
<!-- Include any additional information, caveats, or considerations that
the reviewer should be aware of. -->

***As a contributor to the Anemoi framework, please ensure that your
changes include unit tests, updates to any affected dependencies and
documentation, and have been tested in a parallel setting (i.e., with
multiple GPUs). As a reviewer, you are also responsible for verifying
these aspects and requesting changes if they are not adequately
addressed. For guidelines about those please refer to
https://anemoi.readthedocs.io/en/latest/***

By opening this pull request, I affirm that all authors agree to the
[Contributor License
Agreement.](https://github.com/ecmwf/codex/blob/main/Legal/contributor_license_agreement.md)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
anaprietonem pushed a commit that referenced this pull request Jul 28, 2025
@matschreiner @mchantry @anaprietonem
fix(graphs): throw error instead of raising warning when graph exists. (
2eccc1c 
#379)

## Description

The warning currently thrown by `anemoi-graphs create` when a graph
already exists is too subtle. If you don’t scroll up in a
successful-looking output, you will miss that the graph creation failed.
Here’s an **example output from a failed graph creation** from my
terminal:

![Screenshot from 2025-06-24
08-34-22](https://github.com/user-attachments/assets/0bb36d6e-d488-4689-b527-a73ba1a46ddb)

I began working on a new PR because I thought there was a bug elsewhere,
because I didn’t notice the warning.

To make failure more noisy, I’ve changed the behavior to raise an
explicit error instead of a warning:

![Screenshot from 2025-06-24
08-58-28](https://github.com/user-attachments/assets/96ab03fa-161a-4bfe-bc9f-75976b690778)


 ##
<!-- Include any additional information, caveats, or considerations that
the reviewer should be aware of. -->

***As a contributor to the Anemoi framework, please ensure that your
changes include unit tests, updates to any affected dependencies and
documentation, and have been tested in a parallel setting (i.e., with
multiple GPUs). As a reviewer, you are also responsible for verifying
these aspects and requesting changes if they are not adequately
addressed. For guidelines about those please refer to
https://anemoi.readthedocs.io/en/latest/***

Co-authored-by: Matthew Chantry <[email protected]>
anaprietonem pushed a commit that referenced this pull request Jul 28, 2025
@JPXKQX @pre-commit-ci @anaprietonem
fix(graphs): revert PR #379 (#409)
58d37f2 
## Description
<!-- What issue or task does this change relate to? -->

Revert PR #379 .

Thanks to @havardhhaugen for spotting this.

## What problem does this change solve?
<!-- Describe if it's a bugfix, new feature, doc update, or breaking
change -->

Raising an error during graph creation will break multi-GPU training.
Currently, one limitation of `anemoi-graphs` is that it does not support
distributed creation. For example, during multi-GPU training with
on-the-fly graph creation, the same graph is created on all GPUs. In
this case, raising an error will cause the code to fail on some of the
GPUs.

## What issue or task does this change relate to?
<!-- link to Issue Number -->

##  Additional notes ##
<!-- Include any additional information, caveats, or considerations that
the reviewer should be aware of. -->

***As a contributor to the Anemoi framework, please ensure that your
changes include unit tests, updates to any affected dependencies and
documentation, and have been tested in a parallel setting (i.e., with
multiple GPUs). As a reviewer, you are also responsible for verifying
these aspects and requesting changes if they are not adequately
addressed. For guidelines about those please refer to
https://anemoi.readthedocs.io/en/latest/***

By opening this pull request, I affirm that all authors agree to the
[Contributor License
Agreement.](https://github.com/ecmwf/codex/blob/main/Legal/contributor_license_agreement.md)

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
anaprietonem pushed a commit that referenced this pull request Aug 4, 2025
@DeployDuck
chore: Release main (#380)
9e7f90d 
🤖 Automated Release PR

This PR was created by `release-please` to prepare the next release.
Once merged:

1. A new version tag will be created
2. A GitHub release will be published
3. The changelog will be updated

Changes to be included in the next release:
---


<details><summary>training: 0.6.0</summary>

##
[0.6.0](training-0.5.1...training-0.6.0)
(2025-08-01)


### ⚠ BREAKING CHANGES

* for schemas of data processors
([#433](#433))
* BaseGraphModule and tasks introduced in anemoi-core
([#399](#399))

### Features

* Add metadata back to pl checkpoint.
([#303](#303))
([0193b28](0193b28))
* BaseGraphModule and tasks introduced in anemoi-core
([#399](#399))
([f8ab962](f8ab962))
* **deps:** Use mlflow-skinny instead of mlflow
([#418](#418))
([6a8beb3](6a8beb3))
* Log FTT2 loss + Fourier Correlation loss
([#148](#148))
([345b0ab](345b0ab))
* **model:** Postprocessors for leaky boundings
([#315](#315))
([b54562b](b54562b))
* **models:** Checkpointed Mapper Chunking
([#406](#406))
([8577772](8577772))
* **models:** Mapper edge sharding
([#366](#366))
([326751d](326751d))
* Variable filtering
([#208](#208))
([fba5e47](fba5e47))


### Bug Fixes

* Dropping 3.9 ([#436](#436))
([f6c0214](f6c0214))
* For schemas of data processors
([#433](#433))
([539939b](539939b))
* Mlflow hp params limit
([#424](#424))
([138bc3a](138bc3a))
* Mlflowlogger duplicated key
([#414](#414))
([cb64a1c](cb64a1c))
* **models,traininig:** Hierarchical model + integration test
([#400](#400))
([71dfd89](71dfd89))
* **models:** Add removed sharded_input_key in PR400
([#425](#425))
([089fe6f](089fe6f))
* New checkpoint
([#445](#445))
([a25df93](a25df93))
* Plotting error when precip related params are not diagnostic
([#369](#369))
([010cfa3](010cfa3))
* **training:** Address issues with
[#208](#208)
([#417](#417))
([665f462](665f462))
* **training:** Scaler memory usage
([#391](#391))
([a9d30e1](a9d30e1))
* Update import mflow utils unit tests
([#427](#427))
([70ecdd9](70ecdd9))
* Update level retrieval logic
([#405](#405))
([f393bc3](f393bc3))
* Use transforms: Variable for ExtractVariableGroupAndLevel
([#321](#321))
([7649f4f](7649f4f))
* Warm restart ([#443](#443))
([ff96236](ff96236))


### Documentation

* **graphs:** Documenting some missing features
([#423](#423))
([8addbd8](8addbd8))
</details>

<details><summary>graphs: 0.6.3</summary>

##
[0.6.3](graphs-0.6.2...graphs-0.6.3)
(2025-08-01)


### Features

* **graphs:** Add lat weighted attribute
([#223](#223))
([5dd32ca](5dd32ca))
* **graphs:** Support to export edges to npz
([#395](#395))
([e21738f](e21738f))


### Bug Fixes

* Dropping 3.9 ([#436](#436))
([f6c0214](f6c0214))
* **graphs:** Revert PR
[#379](#379)
([#409](#409))
([d51219f](d51219f))
* **graphs:** Throw error instead of raising warning when graph exists.
([#379](#379))
([6ec6c18](6ec6c18))
* **graphs:** Undo masking when torch-cluster is installed
([#375](#375))
([9f75c06](9f75c06))


### Documentation

* **graphs:** Documenting some missing features
([#423](#423))
([8addbd8](8addbd8))
</details>

<details><summary>models: 0.9.0</summary>

##
[0.9.0](models-0.8.1...models-0.9.0)
(2025-08-01)


### ⚠ BREAKING CHANGES

* for schemas of data processors
([#433](#433))

### Features

* **model:** Postprocessors for leaky boundings
([#315](#315))
([b54562b](b54562b))
* **models:** Checkpointed Mapper Chunking
([#406](#406))
([8577772](8577772))
* **models:** Mapper edge sharding
([#366](#366))
([326751d](326751d))


### Bug Fixes

* Dropping 3.9 ([#436](#436))
([f6c0214](f6c0214))
* For schemas of data processors
([#433](#433))
([539939b](539939b))
* **models,traininig:** Hierarchical model + integration test
([#400](#400))
([71dfd89](71dfd89))
* **models:** Remove repeated lines
([#377](#377))
([1f0b861](1f0b861))
* **models:** Uneven channel sharding
([#385](#385))
([dd095c4](dd095c4))
* Pydantic model validator not working in transformer schema
([#422](#422))
([42f437a](42f437a))
* Remove dead code and fix typo
([#357](#357))
([8c615ba](8c615ba))
</details>

---
> [!IMPORTANT]
> Please do not change the PR title, manifest file, or any other
automatically generated content in this PR unless you understand the
implications. Changes here can break the release process.
> 
> ⚠️ Merging this PR will:
> - Create a new release
> - Trigger deployment pipelines
> - Update package versions

 **Before merging:**
 - Ensure all tests pass
 - Review the changelog carefully
 - Get required approvals

[Release-please
documentation](https://github.com/googleapis/release-please)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mchantry mchantry mchantry approved these changes

Assignees

No one assigned

Labels

contributor graphs

Projects

Anemoi-dev
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matschreiner @mchantry