Graphs Management

[GiovanniCanali]

I'd like to bring up the current approach to graph management in PINA, as I believe it is somewhat complex and could be improved.

Currently, with PINA's Graph, we create an object that internally manages a list of graphs. While this works well, it can make it harder for users to inspect and freely customize individual graphs. Additionally, this approach requires several internal checks for dimensions and consistency. Finally, since PINA's Graph is later transformed into a pyg Batch object, the way data is handled becomes less transparent.

Instead of managing multiple graphs within a single object, I propose handling graphs separately. Specifically, we could create a wrapper class around pyg's torch_geometric.data.Data to manage single graphs. This would allow users to either:

• Provide the necessary values to construct a PINA Graph, or
• Directly pass a torch_geometric.data.Data object, which would then be converted into a PINA Graph.

If multiple graphs are needed, users can instantiate several instances accordingly.

For conditions, we could introduce a GraphCondition class that takes only the input (since the output, if any, is typically stored as a graph attribute, e.g., graph.y). This input could be either a single graph or a list of graphs. If it’s a list, the graphs are grouped into a Batch object, as it is currently done.

This change could bring several advantages:

• Greater control over graphs for users, enabling easier customization.
• Increased transparency in data handling.
• Fewer internal checks, as they would be handled by pyg.
• Support for graph attributes with more than two dimensions, e.g., features of shape [N_nodes, N_params, N_values] for user-defined models or solvers. This is currently not achievable, due to internal checks on shapes.

Notice that while the current logic is somewhat preserved, we would only require fewer checks.

I'm curious to hear your thoughts on this approach. Would this make working with graphs in PINA more intuitive?

[FilippoOlivo]

Great idea! It simplifies a lot the logic and allow use to define more flexible graph object. Actually, I have an idea in mind of the possible behavior of the new class.
Single Graph creation (with KNNGraph and RadiusGraph I expect the same logic)

x = torch.rand((10,4))
y = torch.rand((10,4))
pos = torch.rand((10,3))
edges = torch.tensor([[0,1,2,3,4,5,6,7,8,9],[1,2,3,4,5,6,7,8,9,0]]) 
g = Graph(x=x pos=pos, edge_index=edges, y=y) # This return a torch_geometric.data.Data object

Multi graph creation with the same structure

x = torch.rand((10, 10,4))
y = torch.rand((10,4))
pos = torch.rand((10,3))
edges = torch.tensor([[0,1,2,3,4,5,6,7,8,9],[1,2,3,4,5,6,7,8,9,0]]) 
g = Graph(pos=pos, edge_index=edges) # This return a Graph object which can be used to create torch_geometric.data.Data objects with the same edge_index and pos
data - [g(x=x[i], y=y) for i in range(len(x))] # g(x=x[i], y=y) returns a torch_geometric.data.Data object

If you are agree with the logic proposed by @gc031298 we can introduce it before merging #459

[GiovanniCanali]

@FilippoOlivo I like the proposed approach, as it automates the creation of graphs sharing the same structure. However, to maintain clarity, I would suggest using two separate classes:

• Graph to create the individual graph.
• GraphCreator (or another name that better fits) to handle the creation of multiple graphs sharing the same structure.

What do you think?

[FilippoOlivo]

Sure! This is could be an approach. However, it can lead to an higher number of classes (to be consistent we should define also a RadiusGraphCreator in addition to RadiusGraph). I do not know which is the best solution

[ndem0]

I don't like the GraphCreator actually. If users can employ simple list comprehension to build many graphs, why should we introduce another class?

[FilippoOlivo]

Sure, as for now we have implemented a simpler strategy without a GraphCreator class. However, we have a GraphBuilder class which I want to make abstract. This class only allows to define only once method in common between RadiusGraph and KNNGraph. What do you think?

[dario-coscia]

I think a possible solution would be to simply use Data classes from pyG (a wrapper on it is fine). For radius and knn graph we simply check: if user passes the edge_index we throw an error, otherwise we build internally the graph using pos.

For the conditions I would avoid GraphCondition, since we risk to not use many solvers! I would add in Condition the possibility to pass input_graph and output_graph an iterable of Data or a single Data. Then we can mix (input_points/output_graph, ...) this allows to use freely the solvers.

[FilippoOlivo]

The only thing we need to do for enabling a new solver to work with graph should be adding GraphCondition into accepted_condition_types. This makes possibile to have a certain degree of control on whether a solver can use graphs or not. Additionally a GraphCondition allows to use the internal logic of pyG for internal function such as getitem and batching

[dario-coscia]

I am not saying to not creating a factory (as we do no e.g. InputOutputCondition) I am just saying to keep additional kwargs in Condition which are input_graph, output_graph.

[dario-coscia]

Also even a simple SupervisedSolver would break now since we only accept input_points, output_points (

PINA/pina/solver/supervised.py

Line 97 in 0c8fbdd

input_pts, output_pts = (

). Maybe we can start thinking to call it input and target (as in pytorch).

[FilippoOlivo]

Sure, changing the names should be the best option. However, I was thinking about making the GraphCondition compatible with the already implemented solver by simply adding a function which split Graph object (for example in input and output) before passing data to the solvers. in this way we are fully compatible with the solvers but the user interface for graph management is in "pyG style"

[dario-coscia]

I think this discussion can be closed, as graphs where implemented in #403 and simplified using the graph condition in #459 #471