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

Inspection tools#22

Merged
JPXKQX merged 71 commits intodevelopfrom
feature/inspection_tool
Aug 21, 2024
Merged

Inspection tools#22
JPXKQX merged 71 commits intodevelopfrom
feature/inspection_tool

Conversation

@JPXKQX
Copy link
Copy Markdown
Member

@JPXKQX JPXKQX commented Jul 25, 2024

This PR includes new features for inspecting the graphs:

  • Plots: Interactive plots of the graphs. Distribution plots of edge & node attributes & isolated nodes.
  • Graph description in the console
  • CLI entry point: anemoi-graphs inspect graph.pt ...

Fixes:

  • Argument path in GraphCreator is no longer mandatory.

This work extends the plotting functions created initially by @mishooax, @ssmmnn11 and the rest of the AIFS team.

theissenhelen and others added 30 commits June 24, 2024 14:29
…ecmwf/anemoi-graphs into feature/inspection_tool
Hotfix: nodes config name

Co-authored-by: <[email protected]>
Co-authored-by: <[email protected]>
@JPXKQX
Copy link
Copy Markdown
Member Author

JPXKQX commented Aug 15, 2024

I have left some comments in the PR, @JPXKQX pls feel free to reach out if something is not clear! Great work and really nice feature! I had a few general questions/future ideas:

  • Do we have in mind adding inspection functionality for the edge attributes? In the tables I can see we have area_weights for the nodes, and edge length/directions so was wondering if we could also inform about the shape or visually inspect them.
  • When generating the plots with 'plot_interactive_subgraph', the slider shows the level of connectivity. I was wondering if we could log some information about the values recorded so we know the distribution of the node connections for hidden->hidden, data->hidden and hidden-> data. It could be either some logs prints or a plot of the adjancecy matrix (or that's too big to plot 😅)?
  • Currently, the user has the option of plotting the distribution of these edge attributes. There are 2 main problems with including them in the interactive plots: it scales worse (as there are more number of edges than nodes) and the resulting plot may not be as visually pleasing as a lot of edges may cross.
  • That is a great idea! We can include both print statements in GraphDescriptor and distribution plots in GraphInspector.

@JPXKQX
Copy link
Copy Markdown
Member Author

JPXKQX commented Aug 16, 2024

Thanks for all the feedback @anaprietonem ! We have tried to address all the feedback below:

I found the output tables nice and informative, but I will try to add a table tittle or some additional prints to inform first table is the node summary and second one is about edge summary. And (this might be a naive question) but for the edges summary, the attributes dimension is reported as 3 but then the list of attributes just show two values?

The table titles have been added to the logged report. The attribute dimension is 3 because the edge_dirs is 2D. We could split the attribute dimension by attribute, but that might add too much information to the report, what do you think?

I also test the option to deactivate description/overwriting. How do we plan to document this? maybe an option could be to use the README.md. I am asking this since, I found it not really intuitive that when passing the --description flag then the description is deactivated 😅

Good point! It is now fixed. The user can pass "--description" (default) to leave the option activated or "--no-description" to deactivate it. This is documented in 'anemoi-graphs inspect --help' and will also be included in the official documentation when PR #19 is merged.

The command allows to activate/deactivate --show_attribute_distributions, is there a similar option to be used with --show-nodes? (otherwise by default is always False)

Added new options "--show_nodes" and "--no-show_nodes" (default) to control the interactive node plots.

If I change to True (show_nodes: Optional[bool] = True,) then I hit the following error:

I cannot reproduce this error now.

PD. An additional table has been added for printing the attribute statistics

@anaprietonem
Copy link
Copy Markdown
Contributor

anaprietonem commented Aug 21, 2024

Great thanks for the effort @theissenhelen and @JPXKQX! tested the updated code and:

  • New cli arguments look good
  • I like the new plots to inform about connectivity and table for statistics summary for attribute distributions
  • About the dimensionality and plotting of the edge directions, I would maybe try then to indicate in the edges summary table under 'Attributes' columns something like: edge_length(1), edge_dirs(2) or either: edge_dirs_0,edge_dirs_1, edge_length (same names as the one showed in the 'edge attributes distribution figures'). And thanks for clarifying the point about visually plotting that information, I now understand and agree with that.
  • Regarding the error I still see it, so let me see if I can try to explain how to reproduce it, the exact command is:
    anemoi-graphs inspect --show_nodes graph.pt output_plots
    Since by default we have --no_show_nodes I wanted to test setting this to True.

Screenshot 2024-08-21 at 10 30 20

I can share via Teams the exact recipe.yaml and graph.pt to better reproduce this!

@anaprietonem
Copy link
Copy Markdown
Contributor

Discussed this on Teams, and the issues I reported is now fixed. Great work both of you, my suggestion and comments are now solved, so happy to approve this PR!

@anaprietonem anaprietonem self-requested a review August 21, 2024 12:46
@JPXKQX JPXKQX merged commit 78eda56 into develop Aug 21, 2024
@JPXKQX JPXKQX deleted the feature/inspection_tool branch August 21, 2024 13:09
@JPXKQX JPXKQX restored the feature/inspection_tool branch August 21, 2024 13:15
JPXKQX added a commit that referenced this pull request Aug 21, 2024
@JPXKQX JPXKQX deleted the feature/inspection_tool branch August 23, 2024 07:38
theissenhelen added a commit that referenced this pull request Sep 11, 2024
* Fix (global version) (#12)

* Global Encoder-Processor-Decoder graph (#9)

* feat: Initial implementation of global graphs + fixes

Co-authored by: Mario Santa Cruz <[email protected]>
Co-authored-by: Helen Theissen <[email protected]>
Co-authored-by: Sara Hahner <[email protected]>
Co-authored-by: Jesper Dramsch <[email protected]>

* fix: attributes as torch.float32

* new test: attributes must be float32

* fix typo

* Homogeneize base builders

* improve test docstrings

* homogeneize (name as class attribute)

* new input config

* new default

* remove dataclass from attribute classes

* fix: config nodes name

* 6 generate graphs from icosahedral meshes (#11)

* Global Encoder-Processor-Decoder graph (#9)

* feat: Initial implementation of global graphs

Co-authored by: Mario Santa Cruz <[email protected]>
Co-authored-by: Helen Theissen <[email protected]>
Co-authored-by: Jesper Dramsch <[email protected]>

* fix: attributes as torch.float32

* new test: attributes must be float32

* fix typo

* Homogeneize base builders

* improve test docstrings

* homogeneize (name as class attribute)

* new input config

* new default

* feat: Initial implementation of global graphs

Co-authored by: Mario Santa Cruz <[email protected]>

* add cli command

* Ignore .pt files

* run pre-commit

* docstring + log erros

* initial tests

* feat: initial version of AttributeBuilder

* refactor: separate into node edge attribute builders

* feat: edge_length moved to edges/attributes.py

* remove __init__

* bugfix (encoder edge lengths) + refector

* feat: support path and dict for `config` argument

* fix: error

* refactor: naming

* fix: pre-commit

* feat: builders icosahedral

* feat: Add icosahedral graph generation

Co-authored-by: Mario Santa Cruz <[email protected]>

* refactor: remove create_shere

* feat: Icosahedral edge builder

* feat: hexagonal graph generation

Co-authored-by: Mario Santa Cruz <[email protected]>

* feat: hexagonal builders

* fix: AOI not implemented yet

* fix: abstractmethod and renaming

* chore: add dependencies

* test: add tests for trimesh

* test: add tests for hex (h3)

* fix: imports

* fix: output type

* refactor: delete unused file

* refactor: renaming and positioning

* feat: ensure src and dst always the same

* fix: imports

* fix: edge_name not supported

* test: add tests for TriIcosahedralEdges

* fix: assert missing for Hexagonal edges

* test: hexagonal edges

* fix: avoid same name

* fix: imports

* fix: conflicts

* update tests

* Include xhops to hexagonal edges

* docs: update docstrings

* fix: update attribute name

* refactor: rename multiscale nodes

* refactor: rename icosahedral nodes

* improve: clarity of function

* improve: function syntax

* refactor: simplify resolution assignment

* refactor: improve variable naming in icosahedral graph generation

* more comments

* refactor: naming

* refactor: separate into functions

* refactor: remove unused code

* refactor: remove unused options and rename

* doc: clarify cells and nodes

* fix: add return statements

* homogeneize: tri & hex edges

* naming: xhops to x_hops

* naming: x_hops in docstring

* docstring

* blank lines

* fix: add return statement

* simplify icoshaedral edges

* feat: select edge builder method based on ico node type

* test: adjust tests to new MultiScaleEdges

* docs: improve docstrings

* fix: remove LAM filtering for icosahedral, leave for next PR

* h3 (v4) not supported

---------

* [feature] add changelog (#21)

* ci: changelog check

* feat: add changelog

* fix: on develop

* Skip no-commit-to-branch in code QA (#17)

* Clean nodes after building the graph (#23)

* feat: clean graph of unneeded attributes after creation

Co-authored-by: Mario Santa Cruz <[email protected]>
Co-authored-by: Helen Theissen <[email protected]>
Co-authored-by: Jesper Dramsch <[email protected]>

* Make graph filename argument optional (#24)

* hotfix: make graph saving optional

Co-authored-by: Jesper Dramsch <[email protected]>
Co-authored-by: Mario Santa Cruz <[email protected]>
Co-authored-by: Helen Theissen <[email protected]>

* doc: update CHANGELOG

Co-authored-by: Jesper Dramsch <[email protected]>

* chore: add rtd pr preview

* HEALPix nodes (#16)

[feature] HEALPix node builder

Co-authored-by: theissenhelen <[email protected]>

* 25 integrate reusable workflows (#26)

* ci: add public pr label

* ci: add downstream-ci workflow

* ci: add ci-config

* fix: pre-commit

* docs: update changelog

* ci: switch main on downstream-ci

* ci: run ci on push on main

* Hotfix/clean graph (#28)

* Add binary dependencies to ci-config.yml (#29)

* Add binary dependencies to ci-config.yml

* docs: update changelog

---------

Co-authored-by: theissenhelen <[email protected]>

* ci: inherit pypi publish flow (#27)

* ci: inherit pypi publish flow

Co-authored-by: Jesper Dramsch <[email protected]>

* docs: update changelog

* ci: only downstream-ci for changes in src and tests

---------

Co-authored-by: Jesper Dramsch <[email protected]>

* Bugfix/31 ensure latlon input numpy array (#32)

* #31 Ensure lat lon values passed as numpy arrays

* fix formatting

* Updated CHANGELOG.md

* Remove bug in the graph cleaning function (#33)

* fix: remove the bug in the graph cleaning function

Co-authored-by: Jesper Dramsch <[email protected]>

* docs: update changelog

---------

Co-authored-by: Jesper Dramsch <[email protected]>
Co-authored-by: Gert Mertes <[email protected]>

* Move to reusable workflows for QA and Testing (#34)

* ci: move to reusable workflows for QA and Testing

* docs: changelog add ci reusable workflow

* ci: downstream ci ignore docs

* ci: remove anchor, unsupported by github

* ci: add changelog release updater (#35)

* ci: changelog updater PR (#36)

* ci: correct inputs typo

* ci: fix changelog updater

* docs: ci fixes changelog

* ci: premissions

* ci: typo

* Ci/fix pr permission (#37)

* ci: add pull-requests permissions

* ci: add on workflow dispatch

* ci: remove 3.9 tests (#39)

* ci: remove 3.9 tests

* ci: remove testing of python 3.9

* dics: update changelog

* Fix support for Python 3.9 (#38)

* fix: support py3.9

Co-authored-by: Jesper Dramsch <[email protected]>

* fix: update & homogeneize changelog

* fix: update tests to support py39

* fix: style

* ci: rollback 3.9 tests

* refactor: annotations not used in tests

---------

Co-authored-by: Jesper Dramsch <[email protected]>

* Ci/fix_double_notes (#40)

* ci: avoid duplication of release notes

* ci: try write-all

* ci: fix permissions

* docs: update changelog

* ci: fix indent

* Inspection tools (#22)


Co-authored-by: Ana Prieto Nemesio <[email protected]>
Co-authored-by: Helen Theissen <[email protected]>

* [pre-commit.ci] pre-commit autoupdate (#41)

updates:
- [github.com/psf/black-pre-commit-mirror: 24.4.2 → 24.8.0](psf/black-pre-commit-mirror@24.4.2...24.8.0)
- [github.com/astral-sh/ruff-pre-commit: v0.4.6 → v0.6.2](astral-sh/ruff-pre-commit@v0.4.6...v0.6.2)
- [github.com/tox-dev/pyproject-fmt: 2.1.3 → 2.2.1](tox-dev/pyproject-fmt@2.1.3...2.2.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#43)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.2 → v0.6.3](astral-sh/ruff-pre-commit@v0.6.2...v0.6.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* First version of documentation (#18)

Documentation web page
---------
Co-authored-by: Helen Theissen <[email protected]>
Co-authored-by: Jesper Dramsch <[email protected]>

* [Changelog] Update to 0.3.0 (#44)

* [create-pull-request] automated change

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: theissenhelen <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: Mario Santa Cruz <[email protected]>
Co-authored-by: Mario Santa Cruz <[email protected]>
Co-authored-by: Jesper Dramsch <[email protected]>
Co-authored-by: Gert Mertes <[email protected]>
Co-authored-by: Mario Santa Cruz <[email protected]>
Co-authored-by: Iain Russell <[email protected]>
Co-authored-by: Rilwan (Akanni) Adewoyin <[email protected]>
Co-authored-by: Ana Prieto Nemesio <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: theissenhelen <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature] Inspection tool

4 participants