-
-
Notifications
You must be signed in to change notification settings - Fork 597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimization of all_paths() current impementation, Implementation of all_cycles(), detect_cycles(), display_graph() in the sage's Graph Module #39866
base: develop
Are you sure you want to change the base?
Conversation
…mentations of custom_plot(), find_all_cycles_parallel() method to find all cycles in a graph, digraph_detect_cycle() function
@dcoudert sir can you please review my ideas and implementations |
|
||
The loops and the multiedges if present in the given graph are ignored and | ||
only minimum of the edge labels is kept in case of multiedges. | ||
[Previous docstring remains exactly the same until EXAMPLES] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this, command produced by AI not being correctly interpreted?
path.pop() | ||
visited.remove(node) | ||
|
||
with concurrent.futures.ThreadPoolExecutor() as executor: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a lot of explanation why there's no race condition here. You're modifying shared variable visited
in parallel in multiple threads.
(Also need a lot of explanation why this can be faster when Python GIL is in effect. Is it?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree there might be race conditions because I’m starting DFS from multiple threads and sharing the same set. I used a lock while inserting, but I understand that might not fully solve the problem because of possible rehashing inside .
If needed, I can update it to collect cycles separately per thread and merge them later safely, or I can just run DFS in a single thread to avoid any race conditions completely. Thanks for pointing it out!
Yeah, I know Python has a GIL, so threads don't run in full parallel.
But even with GIL, threads still help. In my code, each DFS can move forward separately, and Python can switch between threads.
Threads also make the code cleaner — like, instead of doing one DFS after another, I can organize them better.
Plus, since there’s a UI (Docker visualization), using threads makes it smoother and more responsive.
Speed wasn’t my main goal here. If we later need to make it super fast, I can shift to multiprocessing.
But for now, threading makes it clean and it works fine.
I'm not sure what to do with this patch-bomb. You try to modify too many different things without considering why some methods are currently implemented this way. Propositions for improvements/modifications must be done in dedicated PRs focusing on a specific change. |
@dcoudert If possible, could you please test the Docker image (UI) for graph visualization and other features? If it’s not convenient, you can also check the results and the individual code for each algorithm here:(https://github.com/PradyumnaPrahas2/GraphAlgorithms-Cpp/tree/master) in screenshots folder. |
1. Optimized Path Finding with Enhanced Usability
Context
This PR enhances SageMath's path finding capabilities by:
shortest_simple_paths()
(Yen's algorithm)all_paths()
functionall_paths(G, start, end, use_multiedges=False, report_edges=False, labels=False, method='default', k=None)
Key Improvements
max_paths=100
parameterBackward Compatibility
✔️ Fully maintains
all_paths()
behavior✔️ Preserves all existing parameters (
use_multiedges
,report_edges
, etc.)✔️ Passes all current tests (including edge cases from #27501)
Synergy with
all_paths()
🚀 Performance Benchmarks
all_paths()
(ms)shortest_simple_paths()
(ms)📊 Visual Summary:
🔄 Feature: Cycle Detection Enhancements in Graphs
🧠 Summary
This PR introduces two powerful methods for cycle detection in graphs, extending
DiGraph
capabilities with performance improvements and flexible APIs.2.
find_all_cycles_parallel(edges, num_nodes)
Purpose:
Detects all unique cycles in undirected graphs using parallel DFS.
Highlights:
ThreadPoolExecutor
Example:
🔄 3.
detect_cycle(method="dfs")
- Smart Cycle Detection🎯 Purpose
Flexible cycle detection with algorithm selection for different graph types and performance needs.
🧠 Supported Methods
dfs
union_find
vertex_coloring
bfs
tarjan
💻 Usage
🔑 Key Insights:
4. Inject plot() Method into Sage's DiGraph Class:
Summary
This PR introduces a new
.plot()
method to theDiGraph
class, enabling users to visualize directed graphs using Matplotlib and NetworkX, rendered inline (e.g., in Jupyter notebooks or SageMathCell environments).Features
.plot()
directly to theDiGraph
classIPython.display.Image
BytesIO
🛠️ Usage Example
Additional Task
🌐 Live Demo Frontend + Graph Algorithm Enhancements
🚀 Interactive UI Showcase
I've developed a full-stack web interface to demonstrate all graph algorithms in action so that it will be easy for the mentor to visualize my work. The UI provides:
🐳 Quick Deployment (2 minutes)
Test the complete system locally using Docker: