Optimize MatchingCoveredGraph’s add_edge method #39678

mashraf8 · 2025-03-11T21:06:55Z

The add_edge method has been optimized to ensure that edges are added while preserving the matching-covered property without fully reinitializing or creating a new graph.

First, the edge is added using the base method, then the graph is checked to confirm that it remains matching-covered. If it is no longer valid, the edge is immediately removed to restore the previous state. The matching is updated only if the graph remains valid.

Doctests were successfully executed to verify the correctness of the modifications.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

…ew graph

mashraf8 · 2025-03-11T21:59:34Z

Hi @dcoudert,

I'm new to contributing to SageMath and actively exploring its environment as I prepare for GSoC. My goal is to become a long-term contributor, and I'm gaining familiarity with the codebase through various contributions. Looking forward to your feedback on this PR!

dcoudert · 2025-03-12T18:00:55Z

Thank you for this PR. I think @janmenjayap should check as he is the main contributor of this module.

I realize that we have no control on the algorithm used to check whether the graph is matching covered. It was already the case. May be we could store the parameters used for creating the instance and use them when calling is_matching_covered ?

…ng_covered

mashraf8 · 2025-03-13T15:52:31Z

Hi @dcoudert,

Thank you for your suggestion! I've taken it into account and implemented it. Now, the instance parameters are preserved and used when calling is_matching_covered. This truly ensures better control over the validation process while preventing any contradictions with the instance's settings.

Additionally, storing these parameters provides flexibility for potential future use in other validations or optimizations within the module.

janmenjayap · 2025-03-13T16:16:49Z

Thank you @mashraf8 for this PR.

The way currently the add_edge() has been implemented is with $$\mathcal{O}(|E| \cdot |V|)$$ time and the same space complexity. The current one suggested does not create a new graph, thus does not consume any extra space, but it also (as it was implemented earlier) does a check whether the whole graph is matching covered or not through is_matching_covered(), which is of $$\mathcal{O}(|E| \cdot |V|)$$ time complexity.

I have mistakenly missed/ overlooked a better implementation of add_edge() earlier, which can be achieved at $$\mathcal{O}(|E|)$$ using the existing matching (self.get_matching()). Essentially, since the base graph is matching covered, observe that each edge participates in some perfect matching. We just need to ensure that the new edge added participates in some perfect matching.

Case 1:
If the new edge added is a parallel/ multiple edge, then clearly it is admissible (aka there exists some perfect matching containing this edge). Hence we can directly add that edge using _backend.

Case 2:
If the new edge uv added is a nonparallel edge joining two existing vertices u and v, we can check whether this edge is admissible using M_alternating_even_mark() method by checking whether there exists an M alternating odd uv path starting and ending with matched edges (this will take $$\mathcal{O}(|E|)$$), using self.get_matching() as the reference perfect matching.

janmenjayap · 2025-03-13T16:26:41Z

I realize that we have no control on the algorithm used to check whether the graph is matching covered. It was already the case. May be we could store the parameters used for creating the instance and use them when calling is_matching_covered ?

Hi,
As of now, the perfect matching stored self._matching is sufficient enough for the matching covered check using the algorithm implemented with M_alternating_even_mark() method. This is crucial even for some further methods that are to be implemented subsequently, for instance concerning removable edges etc. The other useful parameters and attributes (some of which might be invariant wrt the specific matching covered graph, for instance number of bricks, number of Petersen bricks, separating cut characteristic, etc.), can be stored subsequently as they appear in the further implementations.

janmenjayap · 2025-03-13T17:27:03Z

src/sage/graphs/matching_covered_graph.py

+            # Store all parameters used in this instance for reference if needed            
+            self._params = {
+                "data": data,
+                "matching": self._matching,
+                "algorithm": algorithm,
+                "solver": solver,
+                "verbose": verbose,
+                "integrality_tolerance": integrality_tolerance,
+                "args": args,
+                "kwds": kwds  
+            }
+


The parameters data, args and kwds are essentially consumed by Graph.__init__() method, while creating an instance of the matching covered graph. Thus, all the information and attributes of those parameters can be directly accessed from self (for example immutability, or edges being weighted).

Other parameters, that are $-$ algorithm, solver, verbose, integrality_tolerance are used for obtaining self._matching, and are not required afterwards.

This commit may be reverted.

cc: @dcoudert.

I agree with you if you use the algorithm you proposed.

janmenjayap · 2025-03-13T18:26:19Z

src/sage/graphs/matching_covered_graph.py

+
+            # Add the new edge to the graph using the original method from the base class
+            super().add_edge(u, v, label=label) 
+            # Check if the graph is still matching covered after adding the edge
+            if not self.is_matching_covered(**{k: self._params[k] for k in ["algorithm", "solver", "verbose","integrality_tolerance"]}):
+                # If it is no longer matching covered, immediately remove the edge to restore the previous state and raise an exception 
+                super().delete_edge(u, v, label=label)  


Suggested change

# Add the new edge to the graph using the original method from the base class

super().add_edge(u, v, label=label)

# Check if the graph is still matching covered after adding the edge

if not self.is_matching_covered(**{k: self._params[k] for k in ["algorithm", "solver", "verbose","integrality_tolerance"]}):

# If it is no longer matching covered, immediately remove the edge to restore the previous state and raise an exception

super().delete_edge(u, v, label=label)

# If (u, v, label) is a multiple edge/ an existing edge

if self.has_edge(u, v):

self._backend.add_edge(u, v, label, self._directed)

return

# Check if there exists an M-alternating odd uv path starting and

# ending with edges in self._matching

from sage.graphs.matching import M_alternating_even_mark

w = next((b if a == u else a) for a, b, *_ in self.get_matching() if u in (a, b))

if v in M_alternating_even_mark(self, w, self.get_matching()):

# There exists a perfect matching containing the edge (u, v, label)

self._backend.add_edge(u, v, label, self._directed)

return

janmenjayap · 2025-03-13T18:27:35Z

src/sage/graphs/matching_covered_graph.py

+            # Update the matching only if the graph remains matching covered after adding the edge
+            self._matching = self.get_matching()
+


Suggested change

# Update the matching only if the graph remains matching covered after adding the edge

self._matching = self.get_matching()

janmenjayap · 2025-03-13T18:28:31Z

src/sage/graphs/matching_covered_graph.py

                raise ValueError('the graph obtained after the addition of '
                                 'edge (%s) is not matching covered'
                                 % str((u, v, label)))


Suggested change

raise ValueError('the graph obtained after the addition of '

'edge (%s) is not matching covered'

% str((u, v, label)))

raise ValueError('the graph obtained after the addition of edge (%s)'

'is not matching covered' % str((u, v, label)))

janmenjayap · 2025-03-13T18:41:38Z

1159 -       else:
1160 -            # At least one of u or v is a nonexistent vertex.
1161 -            # Thus, the resulting graph is either disconnected
1162 -            # or has an odd order, hence not matching covered
1163 -            raise ValueError('the graph obtained after the addition of edge '
1164 -                             '(%s) is not matching covered'
1165 -                             % str((u, v, label)))

Please remove these lines, as this will be taken care by the last commit of raise ValueError(...).

Let me know if any other query is there in this regard.
Thank you.

dcoudert · 2025-03-13T18:48:21Z

It's better to use if self.has_edge(u, v) and not if u in self.neighbors(v):

mashraf8 · 2025-03-14T01:34:38Z

Hi @janmenjayap,

Thank you so much for all your suggestions! I’ve actually been looking into changing is_matching_covered, and your discovery of M_alternating_even_mark was incredibly effective. As for self_params, we don’t need to use it currently, so it has been removed due to the removal of is_matching_covered.

At this last commit, is there anything else you would suggest?

Thanks again!

dcoudert · 2025-03-14T10:58:11Z

I tried this PR and all tests pass.
@janmenjayap, do we have all needed tests to check that the behavior is ok ? I think so, but you should know better.

janmenjayap · 2025-03-15T06:25:12Z

Yes.
We have all the necessary doc tests.

dcoudert

some minor details.

dcoudert · 2025-03-15T08:26:24Z

src/sage/graphs/matching_covered_graph.py

-
-            except Exception:
-                raise ValueError('the graph obtained after the addition of '
+


remove trailing white spaces (this line should be empty)

dcoudert · 2025-03-15T08:26:31Z

src/sage/graphs/matching_covered_graph.py

+                # There exists a perfect matching containing the edge (u, v, label)
+                self._backend.add_edge(u, v, label, self._directed)
+                return
+


remove trailing white spaces (this line should be empty)

dcoudert · 2025-03-15T08:27:01Z

src/sage/graphs/matching_covered_graph.py

+                self._backend.add_edge(u, v, label, self._directed)
+                return
+
+        raise ValueError('the graph obtained after the addition of '


improve alignment like

raise ValueError('the graph obtained after the addition of edge ' '(%s) is not matching covered' % str((u, v, label)))

janmenjayap · 2025-03-15T11:42:10Z

src/sage/graphs/matching_covered_graph.py

+                return
+            # Check if there exists an M-alternating odd uv path starting and


Suggested change

return

# Check if there exists an M-alternating odd uv path starting and

return

# Check if there exists an M-alternating odd uv path starting and

Add an empty line in between.

janmenjayap · 2025-03-15T11:43:25Z

src/sage/graphs/matching_covered_graph.py

+            w = next((b if a == u else a) for a, b, *_ in self.get_matching() if u in (a, b))
+            if v in M_alternating_even_mark(self, w, self.get_matching()):


Suggested change

w = next((b if a == u else a) for a, b, *_ in self.get_matching() if u in (a, b))

if v in M_alternating_even_mark(self, w, self.get_matching()):

w = next((b if a == u else a) for a, b, *_ in self.get_matching() if u in (a, b))

if v in M_alternating_even_mark(self, w, self.get_matching()):

Add an empty line in between.

mashraf8 · 2025-03-15T13:24:47Z

Thank you, @dcoudert and @janmenjayap
This is my first contribution to Sage, and I will make sure to avoid these minor details in the future.
Let me know if there's anything else to improve.

janmenjayap · 2025-03-15T13:37:25Z

No worries, @mashraf8.

Welcome to SageMath.
Hope you have a great journey ahead!

janmenjayap · 2025-03-15T13:39:45Z

Looks good to me.
cc: @dcoudert.

dcoudert

LGTM. Passes all tests on my laptop.

dcoudert · 2025-03-15T16:59:53Z

@vbraun, I cannot launch the CIs myself. I let you launch them if necessary.
I have tested this PR on my laptop and it works well. For me it's good to go.
Thanks.

github-actions · 2025-03-17T14:07:06Z

Documentation preview for this PR (built with commit 820d609; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

sagemathgh-39678: Optimize MatchingCoveredGraph’s add_edge method The `add_edge` method has been optimized to ensure that edges are added while preserving the **matching-covered** property without fully reinitializing or creating a new graph. First, the edge is added using the base method, then the graph is checked to confirm that it remains matching-covered. If it is no longer valid, the edge is immediately removed to restore the previous state. The matching is updated only if the graph remains valid. **Doctests** were successfully executed to verify the correctness of the modifications. ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. URL: sagemath#39678 Reported by: Mohammed Ashraf Reviewer(s): David Coudert, Janmenjaya Panda

Optimize MatchingCoveredGraph’s add_edge method to avoid creating a n…

6645976

…ew graph

github-actions bot added the s: needs review label Mar 11, 2025

dcoudert added the c: graph theory label Mar 12, 2025

Preserve instance parameters and use them for validation in is_matchi…

a666ab2

…ng_covered

Update kwds handling in self._params in MatchingCoveredGraph

85b87ac

janmenjayap reviewed Mar 13, 2025

View reviewed changes

Switch to a more efficient algorithm

eaa4f9f

dcoudert suggested changes Mar 15, 2025

View reviewed changes

janmenjayap reviewed Mar 15, 2025

View reviewed changes

Ensure compliance with python code style

820d609

dcoudert approved these changes Mar 15, 2025

View reviewed changes

dcoudert added s: positive review and removed s: needs review labels Mar 15, 2025

vbraun merged commit dab7799 into sagemath:develop Mar 22, 2025
24 checks passed

github-actions bot removed the s: positive review label Mar 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize MatchingCoveredGraph’s add_edge method #39678

Optimize MatchingCoveredGraph’s add_edge method #39678

mashraf8 commented Mar 11, 2025 •

edited

Loading

mashraf8 commented Mar 11, 2025

dcoudert commented Mar 12, 2025

mashraf8 commented Mar 13, 2025

janmenjayap commented Mar 13, 2025

janmenjayap commented Mar 13, 2025

janmenjayap Mar 13, 2025

dcoudert Mar 13, 2025

janmenjayap Mar 13, 2025 •

edited

Loading

janmenjayap Mar 13, 2025

janmenjayap Mar 13, 2025 •

edited

Loading

janmenjayap commented Mar 13, 2025 •

edited

Loading

dcoudert commented Mar 13, 2025

mashraf8 commented Mar 14, 2025

dcoudert commented Mar 14, 2025

janmenjayap commented Mar 15, 2025

dcoudert left a comment

dcoudert Mar 15, 2025

dcoudert Mar 15, 2025

dcoudert Mar 15, 2025

janmenjayap Mar 15, 2025 •

edited

Loading

janmenjayap Mar 15, 2025

mashraf8 commented Mar 15, 2025

janmenjayap commented Mar 15, 2025

janmenjayap commented Mar 15, 2025

dcoudert left a comment •

edited

Loading

dcoudert commented Mar 15, 2025

github-actions bot commented Mar 17, 2025

		# Update the matching only if the graph remains matching covered after adding the edge
		self._matching = self.get_matching()


		except Exception:
		raise ValueError('the graph obtained after the addition of '

		return
		# Check if there exists an M-alternating odd uv path starting and

		w = next((b if a == u else a) for a, b, *_ in self.get_matching() if u in (a, b))
		if v in M_alternating_even_mark(self, w, self.get_matching()):

Optimize MatchingCoveredGraph’s add_edge method #39678

Optimize MatchingCoveredGraph’s add_edge method #39678

Conversation

mashraf8 commented Mar 11, 2025 • edited Loading

📝 Checklist

mashraf8 commented Mar 11, 2025

dcoudert commented Mar 12, 2025

mashraf8 commented Mar 13, 2025

janmenjayap commented Mar 13, 2025

janmenjayap commented Mar 13, 2025

janmenjayap Mar 13, 2025

Choose a reason for hiding this comment

dcoudert Mar 13, 2025

Choose a reason for hiding this comment

janmenjayap Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

janmenjayap Mar 13, 2025

Choose a reason for hiding this comment

janmenjayap Mar 13, 2025 • edited Loading

Choose a reason for hiding this comment

janmenjayap commented Mar 13, 2025 • edited Loading

dcoudert commented Mar 13, 2025

mashraf8 commented Mar 14, 2025

dcoudert commented Mar 14, 2025

janmenjayap commented Mar 15, 2025

dcoudert left a comment

Choose a reason for hiding this comment

dcoudert Mar 15, 2025

Choose a reason for hiding this comment

dcoudert Mar 15, 2025

Choose a reason for hiding this comment

dcoudert Mar 15, 2025

Choose a reason for hiding this comment

janmenjayap Mar 15, 2025 • edited Loading

Choose a reason for hiding this comment

janmenjayap Mar 15, 2025

Choose a reason for hiding this comment

mashraf8 commented Mar 15, 2025

janmenjayap commented Mar 15, 2025

janmenjayap commented Mar 15, 2025

dcoudert left a comment • edited Loading

Choose a reason for hiding this comment

dcoudert commented Mar 15, 2025

github-actions bot commented Mar 17, 2025

mashraf8 commented Mar 11, 2025 •

edited

Loading

janmenjayap Mar 13, 2025 •

edited

Loading

janmenjayap Mar 13, 2025 •

edited

Loading

janmenjayap commented Mar 13, 2025 •

edited

Loading

janmenjayap Mar 15, 2025 •

edited

Loading

dcoudert left a comment •

edited

Loading