Implementation of DigraphInsertEdge and DigraphReduceEdge by linuskuehnle · Pull Request #899 · digraphs/Digraphs

linuskuehnle · 2026-02-18T16:25:00Z

Implemented DigraphInsertEdge and DigraphReduceEdge.
These functions already exist in the Simplicial Surfaces package, which I am currently working on with Meike Weiß at RWTH Aachen University.
For reference, the corresponding functions there are implemented as EdgeInsertion and EdgeReduction.

Both DigraphInsertEdge and DigraphReduceEdge check whether the given digraph D is symmetric, and modifications to D preserve this symmetry.
Should D therefore retain the symmetric tag by explicitly calling SetIsSymmetricDigraph after modification?
If so, is an additional call to SetDigraphSymmetricClosureAttr required?

…InsertEdge and DigraphReduceEdge

james-d-mitchell · 2026-02-21T19:26:38Z

Thanks for the contribution @linuskuehnle ! I'll try to review your PR and answer your questions early next week.

… edge's vertices having two common neighbours

james-d-mitchell

Generally this looks pretty good, thanks for the contribution @linuskuehnle ! If you could please address the comments, then I'll be happy to merge this.

james-d-mitchell · 2026-02-21T19:24:12Z

+    then then this operation inserts a new edge between <A>edge1</A> and
+    <A>edge2</A>.
+    <A>edge1</A> and <A>edge2</A> are split up into two new edges each, which are
+    all adjacent to the inserted edge. Hence, the vertices of the inserted edge both


What does it mean for an edge to be adjacent to an edge?

By adjacent edges I mean edges that share a common vertex. The operation subdivides edge1 and edge2 by placing a new vertex on each, and then connects those two new vertices with a newly inserted edge. So the four 'split' edge halves share endpoints with the newly inserted edge. Hope i explained it properly now.
Should i rephrase the documentation here?

james-d-mitchell · 2026-03-24T10:19:26Z

+  <Returns>A digraph.</Returns>
+  <Description>
+    If <A>edge1</A> and <A>edge2</A> are distinct pairs of vertices of <A>digraph</A>,
+    then then this operation inserts a new edge between <A>edge1</A> and


Suggested change

then then this operation inserts a new edge between <A>edge1</A> and

then this operation inserts a new edge between <A>edge1</A> and

james-d-mitchell · 2026-03-24T10:19:41Z

+    <A>edge2</A>.
+    <A>edge1</A> and <A>edge2</A> are split up into two new edges each, which are
+    all adjacent to the inserted edge. Hence, the vertices of the inserted edge both
+    have a degree of 3.


Suggested change

have a degree of 3.

have a degree of 3.<P/>

james-d-mitchell · 2026-03-24T10:19:48Z

+    all adjacent to the inserted edge. Hence, the vertices of the inserted edge both
+    have a degree of 3.
+
+    The opposite operation is <Ref Oper="DigraphReduceEdge"/>.


Suggested change

The opposite operation is <Ref Oper="DigraphReduceEdge"/>.

The opposite operation is <Ref Oper="DigraphReduceEdge"/>.<P/>

james-d-mitchell · 2026-03-24T10:21:26Z

+    A new digraph constructed from <A>digraph</A> is returned,
+    unless <A>digraph</A> belongs to <Ref Filt="IsMutableDigraph"/>;
+    in this case changes are made directly to <A>digraph</A>, which is then returned. 
+    The <A>digraph</A> must not belong to <Ref Filt="IsMultiDigraph"/>. <P/>


Maybe say that an error is thrown if IsMultiDigraph is true?

I copied this section from the documentation of DigraphContractEdge. I can mention that an error is thrown in that case, should i then adjust it for DigraphContractEdge as well?

james-d-mitchell · 2026-03-24T10:25:10Z

+
+  DIGRAPHS_CheckInsertEdgeDigraph(D, edge1, edge2);
+
+  numVertices := Maximum(DigraphVertices(D));


Suggested change

numVertices := Maximum(DigraphVertices(D));

numVertices := DigraphNrVertices(D);

james-d-mitchell · 2026-03-24T10:28:16Z

+  return MakeImmutable(D);
+end);
+
+DIGRAPHS_CheckReduceEdgeDigraph := function(D, edge)


Same two comments for this function:

It doesn't need to exist;

it can be shortened a little by combining multiple if statements with elif

james-d-mitchell · 2026-03-24T10:29:55Z

+function(D, edge1, edge2)
+  D := DigraphInsertEdge(DigraphMutableCopy(D), edge1, edge2);
+
+  return MakeImmutable(D);


Suggested change

return MakeImmutable(D);

{D, edge1, edge2} -> MakeImmutable(DigraphInsertEdge(DigraphMutableCopy(D), edge1, edge2)));

james-d-mitchell · 2026-03-24T10:30:51Z

+
+  leftVertexOutNeighbours := OutNeighboursOfVertex(D, edge[1]);
+  rightVertexOutNeighbours := OutNeighboursOfVertex(D, edge[2]);
+  allVertexOutNeighbours := Union(leftVertexOutNeighbours,


I'd move this down after the checks in 713, to avoid computing this in case an error is already given.

james-d-mitchell · 2026-03-24T10:31:47Z

+InstallMethod(DigraphReduceEdge,
+"for a symmetric digraph and a dense list",
+[IsImmutableDigraph, IsDenseList],
+function(D, edge)


Suggested change

function(D, edge)

{D, edge} -> MakeImmutable(DigraphReduceEdge(DigraphMutableCopy(D), edge)));

… of DigraphInsertEdge and DigraphReduceEdge

added implementation, testing and documentation of operations Digraph…

456db83

…InsertEdge and DigraphReduceEdge

linuskuehnle added 2 commits February 26, 2026 11:31

adjusted to formatting enforced by gaplint

c83e29c

adjusted formatting and added DigraphReduceEdge error check regarding…

204d43a

… edge's vertices having two common neighbours

james-d-mitchell requested changes Mar 24, 2026

View reviewed changes

implemented requested changes on documentation and function structure…

cbe1449

… of DigraphInsertEdge and DigraphReduceEdge

	then then this operation inserts a new edge between <A>edge1</A> and
	then this operation inserts a new edge between <A>edge1</A> and

	The opposite operation is <Ref Oper="DigraphReduceEdge"/>.
	The opposite operation is <Ref Oper="DigraphReduceEdge"/>.<P/>


		DIGRAPHS_CheckInsertEdgeDigraph(D, edge1, edge2);

		numVertices := Maximum(DigraphVertices(D));

	numVertices := Maximum(DigraphVertices(D));
	numVertices := DigraphNrVertices(D);

	return MakeImmutable(D);
	{D, edge1, edge2} -> MakeImmutable(DigraphInsertEdge(DigraphMutableCopy(D), edge1, edge2)));

	function(D, edge)
	{D, edge} -> MakeImmutable(DigraphReduceEdge(DigraphMutableCopy(D), edge)));

Conversation

linuskuehnle commented Feb 18, 2026

Uh oh!

james-d-mitchell commented Feb 21, 2026

Uh oh!

james-d-mitchell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants