feat(graphs): support area positional argument in GraphInspector #415
Conversation
for more information, see https://pre-commit.ci
…/ecmwf/anemoi-core into feat/add-area-to-graph-inspector
for more information, see https://pre-commit.ci
| @@ -20,8 +20,8 @@ You can define your graph in a YAML configuration file, following the | |||
| same approach described in the other usage sections. This YAML file | |||
| should specify the nodes, edges, and any attributes required for your | |||
| graph. For more details on how to structure your recipe, refer to the | |||
| examples in the :doc:`usage-getting_started` and | |||
| :doc:`usage-limited_area` sections. | |||
| examples in the :ref:`usage-getting_started` and | |||
There was a problem hiding this comment.
I guess this is fixing a broken link from before in the dosc?
| "Nodes from which to remove the unconnected nodes." | ||
| area: tuple[float, float, float, float] = Field(default=(40, 10, 30, 20)) | ||
| "Area of interest to crop the nodes, (north, west, south, east)." | ||
| nodes_name: str | Iterable[str] |
There was a problem hiding this comment.
I think this is repeated
|
|
||
| # Convert area limits to [-90, 90]x[0,360] | ||
| west, east = west % 360, (east - 1) % 360 + 1 | ||
| assert (west - east) % 360 != 0, "West and East limits must be different." |
There was a problem hiding this comment.
I'd add a few more asserts to ensure values are in the expected limits (-90,90) and (0,360)
|
|
||
| def compute_mask(self, graph: HeteroData, nodes_name: str) -> torch.Tensor: | ||
| """Compute the mask of connected nodes.""" | ||
| latlons = graph[nodes_name].x * 180 / torch.pi |
There was a problem hiding this comment.
could we use np.rad2deg instead?
| def compute_mask(self, graph: HeteroData, nodes_name: str) -> torch.Tensor: | ||
| """Compute the mask of connected nodes.""" | ||
| latlons = graph[nodes_name].x * 180 / torch.pi | ||
| mask = SubsetNodesInArea.points_inside_area(lats=latlons[:, 0], lons=latlons[:, 1], area=self.area) |
There was a problem hiding this comment.
Do you think we will be using this method somewhere else or what's the motivation for making it a staticmethod?
| graph = self.update_edge_indices(graph) | ||
| graph = self.add_attribute(graph) | ||
| for nodes_name in self.nodes_names: | ||
| mask = self.compute_mask(graph, nodes_name) |
There was a problem hiding this comment.
if those are going to be hidden methods and the update_graph is the public one could be make those _ compute_mask for clarity?
| connected_mask[edges.edge_index[1]] = True | ||
|
|
||
| return connected_mask | ||
|
|
||
|
|
||
| class SubsetNodesInArea(BaseNodeMaskingProcessor): |
There was a problem hiding this comment.
could be have a test for this also on graphs/tests/processors/test_post_process.py?
Description
This PR adds the positional argument
areain theGraphInspectorclass. It can also be used in the CLI as follows,What problem does this change solve?
The current inspection tools are great for debugging and designing our graphs, but they lack the ability to support high-resolution graphics.
What issue or task does this change relate to?
Additional notes
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.
📚 Documentation preview 📚: https://anemoi-training--415.org.readthedocs.build/en/415/
📚 Documentation preview 📚: https://anemoi-graphs--415.org.readthedocs.build/en/415/
📚 Documentation preview 📚: https://anemoi-models--415.org.readthedocs.build/en/415/