[WIP] Review and refactor network layout algorithms for accuracy#39
Merged
Conversation
Co-authored-by: Adamtaranto <2160099+Adamtaranto@users.noreply.github.com>
Co-authored-by: Adamtaranto <2160099+Adamtaranto@users.noreply.github.com>
Co-authored-by: Adamtaranto <2160099+Adamtaranto@users.noreply.github.com>
Copilot stopped work on behalf of
Adamtaranto due to an error
November 16, 2025 08:36
Owner
|
@copilot finalise tasks |
Contributor
Author
Tasks finalized and PR is ready for review. ✅ Summary of completed work:
Quality checks:
Key deliverables:
The PR is production-ready and can be merged. (Commits: df1ede3, d17b404, be9a53f) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Network Layout Algorithm Review and Refactoring
Plan
Summary of Changes
1. Fixed Bug in Hierarchical Layout ✅
Issue: Hierarchical layout only computed positions for nodes reachable from root, ignoring disconnected components.
Fix: Added handling for disconnected nodes at max_level + 1, matching RadialLayout behavior.
Impact: All nodes now receive positions, preventing incomplete visualizations.
2. Added Spectral Layout Algorithm ✅
New Feature: Implemented SpectralLayout using graph Laplacian eigenvectors.
Performance Benefits:
3. Added Layout Caching ✅
Feature: LayoutManager now caches layout computations with identical parameters.
Benefits:
enable_cacheparameter)Example:
4. Comprehensive Documentation ✅
New File:
docs/layout_algorithms.md(10KB guide)Contents:
5. Enhanced Code Documentation ✅
6. Updated GUI ✅
Test Results
Performance Comparison
Algorithm Selection Guide
Small networks (<50 nodes): Kamada-Kawai or Force-Directed
Medium networks (50-500 nodes): Force-Directed or Spectral
Large networks (>500 nodes): Spectral or Hierarchical
Fast preview: Hierarchical (instant)
Files Modified
src/pypopart/layout/algorithms.py: Bug fix, new algorithm, caching, docssrc/pypopart/layout/__init__.py: Added SpectralLayout exportsrc/pypopart/gui/app.py: Added spectral option, reordered layoutstests/unit/test_layout.py: 7 new testsdocs/layout_algorithms.md: New comprehensive guideBackwards Compatibility
✅ All changes are backwards compatible:
Original prompt
Review the network layout algorithms for accuracy and efficiency. Refactor if required.
tment of node layout in network graphs.
In geographic layout map sure a base map is displayed under the network graph.
AI: Your work will be continued in this pull request.
User:
When using MJN to calculate network and any epsilon value > 0, get error:
When using MJN, haplotype summary tab information is incorrect. Sample IDs are reported as H numbers or "Median_" numbers i.e. "Median_2"
Sample IDs should correspond to sequence IDs from the input alignment.
Are the sample_IDs begining with "Median_" actually observed haplotypes? or are they inferred by the mjn method?
AI: Your work will be continued in this pull request.
User: Error when using MJN with epsilon > 0:
Also, MJN graph produced with epsilon 0 contain inferred median nodes that do not bridge observed nodes either directly or via other median nodes, should these be preserved in the final network?
Compare and review our pure python implementation of mjn to the original popart C++ implementation
AI: Your work will be continued in this pull request.
User: Metadata template in dash app uses different column headers to what is expected by reader:
In network graphs add edge labels indicating how many changes are required to get from one haplotype node to another in the network.
In alignment view highlight minor alleles or gaps in columns that contain >1 base. Colour using standard nucleotide colour scheme.
AI: Your work will be continued in this pull request.
User: Explain how to manually reposition nodes ...
Created from VS Code via the GitHub Pull Request extension.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.