An EventGraph class with tests

[vineetbansal]

Introduced an EventGraph class with tests to illustrate basic functionality. EventGraph is a pathpyG Graph so in addition to being constructed from a TemporalGraph, it is capable of being constructed directly from a torch_geometric.data.Data object.

Essentially the usage might be:

  tg = TemporalGraph.from_edge_list([
      ("a", "b", 1),
      ("b", "c", 2),
      ("b", "d", 5),
      ("c", "e", 3),
  ])

  eg = EventGraph.from_temporal_graph(tg, delta=2)

  print(eg)                    # EventGraph (delta=2) ... one line per event, like a->b@1
  print(eg.num_events)         # 4  (one per temporal edge)
  print(eg.num_fo_nodes)       # 5  (a, b, c, d, e)

  print(eg.event_endpoints(0)) # ('a', 'b', 1)
  print(eg[0])                 # same thing
  print(eg.continuations(0))   # [(1, 1)]  -> event 1, with a time gap of 1

  dist, pred = eg.shortest_paths()
  print(dist[eg.fo_mapping.to_idx("a"), eg.fo_mapping.to_idx("e")])  # 3.0

  tg_back = eg.to_temporal_graph()
  
  # This is not part of this PR, but can be added easily.
  model = MultiOrderModel.from_event_graph(event_graph, max_order=2)

A couple of things to discuss:

• If an EventGraph is constructed from a TemporalGraph and the TemporalGraph is later changed by the caller, should the EventGraph change as well? Right now we don't - eg._temporal_graph = TemporalGraph(g.data.clone(), mapping=g.mapping). This seems to me like the right thing to do.

• eg.shortest_paths works, but creates a TemporalGraph (if it doesn't exist) to pass on to temporal_shortest_paths. This seems wasteful if EventGraph was constructed from Data and doesn't need all the TemporalGraph machinery. Perhaps temporal_shortest_paths can be tweaked to work with a pre-computed edge-index (as would be the case if EventGraph is constructed directly from torch_geometric.data.Data).

  • Decision - change signature to temporal_shortest_paths(g: TemporalGraph | None, eg: "EventGraph" | None, delta: int). Add methods to EventGraph/TemporalGraph that call this function under the hood.

Some changes in other files were for my understanding of the code by adding comments, and are unrelated to the PR.

[vineetbansal]

Some TODOs that came out of discussion:

• Make lift_order_temporal a method in EventGraph.
• Change signature of temporal_shortest_paths to temporal_shortest_paths(g: TemporalGraph | None, eg: "EventGraph" | None, delta: int). Add methods to EventGraph/TemporalGraph that call this function under the hood.
• Add a reduce_delta method that (smartly) constructs a new EventGraph object. Useful for delta sweeps in MultiOrderModel.
• Avoid using the term continuations + make private.
• Add a mapping attribute (mapping labels like a->b@1 to event ids).

[M-Lampert]

Thanks for keeping track of everything we discussed in the meeting. Some additional suggestions:

• I think we should rename lift_order_temporal to create_event_graph_index or something similar that explains better what the method actually does.
• About continuations: You probably got it from the following line, right?

  pathpyG/src/pathpyG/algorithms/temporal.py

  Line 36 in 8b1c2f4

  # lift order: find possible continuations for edges in each time stamp

  I think we use this functionality only in the event graph construction, and there we cannot make use of this function since we do not have the EventGraph object yet. So, I am not sure if we should add this function at all. Once we are at a stable version, removing it would mean a breaking change. Instead, later adding it would not be a problem if we need it at some point.
• Adding the mapping attribute would make the event_endpoints function almost redundant. Not sure if we should keep the function then.
• Regarding num_fo_nodes, I think I would prefer not having abbreviations in the function names since not all people might now that it refers to "first-order". Additionally, in Graph we use Graph.n as the property to get the number of nodes. We should make this consistent. Maybe n_first_order?

I think these are all things that we need to agree on now since changing them later would mean breaking changes. So, maybe you can also add what you think @hackl and @IngoScholtes

FYI: Just merged the pytest CI fix to main, so after rebasing you shouldn't run into problems anymore.

[vineetbansal]

@M-Lampert - I've moved the lift_order_temporal function to be a @staticmethod build_edge_index in EventGraph so it looks like:

build_edge_index(g: TemporalGraph, delta: float | int = 1):

and returns the exact same thing as before (the ho_index). This hopefully addresses the concern that it needs to live in the EventGraph namespace but be flexible enough to work with any temporal graph or delta passed in.

If not, let me know - happy to move around.

For the purposes of fixing the documentation related to this, let me know if simply renaming lift_order_temporal to EventGraph.build_edge_index but still calling it a "function" (it is functional), instead of a "method" (which might be confusing for the target audience?) in the docs would be acceptable.

Of course this is to get the docs consistent with the PR and get the CI to pass wrt this PR - it might move again in the (near) future.

[M-Lampert]

Just replacing every lift_order_temporal with EventGraph.build_edge_index in the docs and still calling it a "function" seems fine to me.

[vineetbansal]

I've addressed the bullet points above. Main additions since we met:

• Added a MultiOrderModel.from_event_graph method.
• reduce_delta method for EventGraph that returns another EventGraph.

[M-Lampert]

Related to the visualisation issue that I just mentioned at the beginning of our meeting, the issue is caused by the following method:

pathpyG/src/pathpyG/visualisations/network_plot.py

Lines 120 to 146 in 02c0dc6

	def _compute_node_data(self) -> None:
	"""Build node DataFrame with visual attributes.
	Creates indexed DataFrame for all nodes, handling higher-order networks
	by converting tuple nodes to string representation. Assigns attributes
	from config defaults, network data, and user arguments.
	"""
	# initialize values
	nodes: pd.DataFrame = pd.DataFrame(index=self.network.nodes)
	# if higher-order network, convert node tuples to string representation
	if self.network.order > 1:
	nodes.index = nodes.index.map(lambda x: self.config["separator"].join(map(str, x)))
	for attribute in self.attributes:
	# set default value for each attribute based on the pathpyG.toml config
	if isinstance(self.config.get("node").get(attribute, None), list | tuple): # type: ignore[union-attr]
	nodes[attribute] = [self.config.get("node").get(attribute, None)] * len(nodes) # type: ignore[union-attr]
	else:
	nodes[attribute] = self.config.get("node").get(attribute, None) # type: ignore[union-attr]
	# check if attribute is given as node attribute
	if f"node_{attribute}" in self.network.node_attrs():
	nodes[attribute] = self.network.data[f"node_{attribute}"]
	# check if attribute is given as argument
	if attribute in self.node_args:
	nodes = self._assign_argument(attribute, self.node_args[attribute], nodes)
	# save node data
	self.data["nodes"] = nodes

In the method, we check the order of the graph in line 130. Since it correctly detects that the event graph is of order 2, it wants to add an additional separator between the node labels. This is because, as we just discussed, in our current implementation, we use the Graph-class for every graph regardless of order and always save the first-order node labels. Then, for correct visualisation, we needed to modify the node labels on the fly.
I guess this won't be necessary anymore since we will have the dedicated classes for the event, but also the higher-order graph, so that we do not need to make this check anymore. I think if we just remove lines 130-131, everything will work for now and with the HigherOrderGraph coming soon, this should then handle every case we needed these lines for.
Let me know if you have any questions about this.

[vineetbansal]

@M-Lampert - hmm, I think the fix might be upstream of this - by the time I get here, self.network.nodes is already ('c', '-', '>', 'e', '@', '3').

[M-Lampert]

You are right. Line 130 in NetworkPlot is not the only problem. We do a similar check in Graph:

pathpyG/src/pathpyG/core/graph.py

Lines 329 to 342 in 02c0dc6

	def nodes(self) -> list:
	"""Return indices or IDs of all nodes in the graph.
	This method returns a list object that contains all nodes.
	If an IndexMap is used, nodes are returned as string IDs.
	If no IndexMap is used, nodes are returned as integer indices.
	Returns:
	list: list of all nodes using IDs or indices (if no mapping is used)
	"""
	node_list = self.mapping.to_ids(np.arange(self.n)).tolist()
	if self.order > 1:
	return list(map(tuple, node_list))
	return node_list

This is the same underlying problem: All of these classes are implemented to handle higher-order graphs using the Graph-class. I think this part is not documented well but if you create a graph using the constructor and pass a node_sequence of the shape (m, order) with order > 1 like you do in EventGraph.from_temporal_graph(...):

pathpyG/src/pathpyG/core/event_graph.py

Lines 103 to 108 in a0fd316

	data = Data(
	edge_index=ho_index,
	num_nodes=m,
	node_sequence=node_sequence,
	node_time=node_time,
	)

Then Graph will assume that we still have a first-order IndexMap and try to handle all of the higher-order parts on the fly with these checks order > 1.
I think with the coming HigherOrderGraph-class, we can remove all of these checks and design it in a way that the higher-order graph handles this all automatically.

In between stages, I think there is no good solution for this, so maybe we can leave it as it is for now, open an issue for this and then solve it for every higher-order graph (EventGraph and HigherOrderGraph) at the same time.

[M-Lampert]

I opened #323 regarding this and we can come back to it once the HigherOrderGraph is also implemented.
Are there any other points that are still open in the PR? Otherwise I can review it once more and then we can merge.

[M-Lampert]

Sorry, that I keep coming up with new things but I just compared the __str__ here to the one in TemporalGraph. There, we also return information about the node, edge, and graph attributes:

pathpyG/src/pathpyG/core/temporal_graph.py

Lines 346 to 380 in 02c0dc6

	def __str__(self) -> str:
	"""Return a string representation of the graph."""
	s = "Temporal Graph with {0} nodes, {1} unique edges and {2} events in [{3}, {4}]\n".format(
	self.data.num_nodes,
	self.data.edge_index.unique(dim=1).size(dim=1),
	self.data.edge_index.size(1),
	self.start_time,
	self.end_time,
	)
	attr = self.data.to_dict()
	attr_types = {}
	for k in attr:
	t = type(attr[k])
	if t == torch.Tensor:
	attr_types[k] = str(t) + " -> " + str(attr[k].size())
	else:
	attr_types[k] = str(t)
	from pprint import pformat
	attribute_info: dict[str, dict[str, Any]] = {
	"Node Attributes": {},
	"Edge Attributes": {},
	"Graph Attributes": {},
	}
	for a in self.node_attrs():
	attribute_info["Node Attributes"][a] = attr_types[a]
	for a in self.edge_attrs():
	attribute_info["Edge Attributes"][a] = attr_types[a]
	for a in self.data.keys():
	if not self.data.is_node_attr(a) and not self.data.is_edge_attr(a):
	attribute_info["Graph Attributes"][a] = attr_types[a]
	s += pformat(attribute_info, indent=4, width=160)
	return s

Something similar should also be added here. Maybe you can even deduplicate the code in graph and temporal graph since I just saw that they share almost half of the function.

In general, does the event graph construction from a temporal graph currently automatically convert existing attributes in the temporal graph to attributes in the event graph? I.e. edge attributes in the temporal graph should become node attributes in the event graph, node attributes in the temporal graph should be dropped and graph attributes should stay the same as before.

[vineetbansal]

No - currently it doesn't. Good point though, since pathpyG identifies attribute kind by prefix (node/edge). I'll implement this in a separate commit.

[M-Lampert]

Sorry, this is something that we haven't explained properly. __getitem__ is also related to the attributes just like __str__ above. You could for example have an attribute called node_color. When calling event_graph["node_color"] you should get an iterable of the saved colors for each node. I think this is something that should work directly with the implementation from Graph so you might not even need to implement the function here. The only thing that you need to consider is that during the creation of the event graph, node attributes from a temporal graph will be transformed to edge attributes in the event graph.

[M-Lampert]

It would be good if you could test if the attribute handling works properly here.