-
Notifications
You must be signed in to change notification settings - Fork 430
fix: Algos don't work on UndirectedAdaptor (#870)
#871
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
Merged
starovoid
merged 3 commits into
petgraph:master
from
CattleProdigy:undirectedadaptor-fix
Sep 10, 2025
Merged
fix: Algos don't work on UndirectedAdaptor (#870)
#871
starovoid
merged 3 commits into
petgraph:master
from
CattleProdigy:undirectedadaptor-fix
Sep 10, 2025
Conversation
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
Contributor
Author
|
All these lints are slated to be fixed here: #869 |
017304e to
654997f
Compare
starovoid
requested changes
Sep 8, 2025
Collaborator
starovoid
left a comment
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.
Nice, thanks for the bugfix. I only have one suggestion for improving the test.
Closes petgraph#870 The `IntoEdges` implementation for `UnidirectedAdaptor` does not produce the same set of edges references as an equivalently-constructed undirected graph. This prevents search and traversal algorithms from working if the traversal or search would require traversing an adapted edge whose underlying directed edge is "incoming" (which would prevent traversal in the directed case). The underlying reason is that we currently, correctly concatenate incoming and outgoing edges from the underlying directed graph but they're concatenated with the same orientation which produces a set of redundant `EdgeReferences`-- making the adapted graph equivalent to the underlying directed graph as far as the `IntoEdges` implementation is concerned. The `IntoEdges` implementation for undirected graphs produces two `EdgeReference`s for each graph edge-- one for each orientation. This is what allows traversal of edges in either direction. The solution is to make the `UndirectedAdaptor`'s `IntoEdges` behavior match that of an undirected graph by reversing the edges with `Incoming` orientation. That way there will be two `EdgeReferences` for each edge as there are in undirected graphs.
654997f to
b815451
Compare
starovoid
approved these changes
Sep 10, 2025
Merged
RaoulLuque
pushed a commit
to RaoulLuque/petgraph
that referenced
this pull request
Sep 21, 2025
…#871) Closes petgraph#870 The `IntoEdges` implementation for `UnidirectedAdaptor` does not produce the same set of edges references as an equivalently-constructed undirected graph. This prevents search and traversal algorithms from working if the traversal or search would require traversing an adapted edge whose underlying directed edge is "incoming" (which would prevent traversal in the directed case). The underlying reason is that we currently, correctly concatenate incoming and outgoing edges from the underlying directed graph but they're concatenated with the same orientation which produces a set of redundant `EdgeReferences`-- making the adapted graph equivalent to the underlying directed graph as far as the `IntoEdges` implementation is concerned. The `IntoEdges` implementation for undirected graphs produces two `EdgeReference`s for each graph edge-- one for each orientation. This is what allows traversal of edges in either direction. The solution is to make the `UndirectedAdaptor`'s `IntoEdges` behavior match that of an undirected graph by reversing the edges with `Incoming` orientation. That way there will be two `EdgeReferences` for each edge as there are in undirected graphs. <!-- -- Thanks for contributing to `petgraph`! -- -- We require PR titles to follow the Conventional Commits specification, -- https://www.conventionalcommits.org/en/v1.0.0/. This helps us generate -- changelogs and follow semantic versioning. -- -- Start the PR title with one of the following: -- * `feat:` for new features -- * `fix:` for bug fixes -- * `refactor:` for code refactors -- * `docs:` for documentation changes -- * `test:` for test changes -- * `perf:` for performance improvements -- * `revert:` for reverting changes -- * `ci:` for CI/CD changes -- * `chore:` for changes that don't fit in any of the above categories -- The last two categories will not be included in the changelog. -- -- If your PR includes a breaking change, please add a `!` after the type -- and include a `BREAKING CHANGE:` line in the body of the PR describing -- the necessary changes for users to update their code. -- --> --------- Co-authored-by: Paul Jakob Schroeder <paul.schroeder@tangramvision.com>
RaoulLuque
pushed a commit
to cactusdualcore/petgraph
that referenced
this pull request
Sep 21, 2025
…#871) Closes petgraph#870 The `IntoEdges` implementation for `UnidirectedAdaptor` does not produce the same set of edges references as an equivalently-constructed undirected graph. This prevents search and traversal algorithms from working if the traversal or search would require traversing an adapted edge whose underlying directed edge is "incoming" (which would prevent traversal in the directed case). The underlying reason is that we currently, correctly concatenate incoming and outgoing edges from the underlying directed graph but they're concatenated with the same orientation which produces a set of redundant `EdgeReferences`-- making the adapted graph equivalent to the underlying directed graph as far as the `IntoEdges` implementation is concerned. The `IntoEdges` implementation for undirected graphs produces two `EdgeReference`s for each graph edge-- one for each orientation. This is what allows traversal of edges in either direction. The solution is to make the `UndirectedAdaptor`'s `IntoEdges` behavior match that of an undirected graph by reversing the edges with `Incoming` orientation. That way there will be two `EdgeReferences` for each edge as there are in undirected graphs. <!-- -- Thanks for contributing to `petgraph`! -- -- We require PR titles to follow the Conventional Commits specification, -- https://www.conventionalcommits.org/en/v1.0.0/. This helps us generate -- changelogs and follow semantic versioning. -- -- Start the PR title with one of the following: -- * `feat:` for new features -- * `fix:` for bug fixes -- * `refactor:` for code refactors -- * `docs:` for documentation changes -- * `test:` for test changes -- * `perf:` for performance improvements -- * `revert:` for reverting changes -- * `ci:` for CI/CD changes -- * `chore:` for changes that don't fit in any of the above categories -- The last two categories will not be included in the changelog. -- -- If your PR includes a breaking change, please add a `!` after the type -- and include a `BREAKING CHANGE:` line in the body of the PR describing -- the necessary changes for users to update their code. -- --> --------- Co-authored-by: Paul Jakob Schroeder <paul.schroeder@tangramvision.com>
RaoulLuque
pushed a commit
to RaoulLuque/petgraph
that referenced
this pull request
Sep 21, 2025
…#871) Closes petgraph#870 The `IntoEdges` implementation for `UnidirectedAdaptor` does not produce the same set of edges references as an equivalently-constructed undirected graph. This prevents search and traversal algorithms from working if the traversal or search would require traversing an adapted edge whose underlying directed edge is "incoming" (which would prevent traversal in the directed case). The underlying reason is that we currently, correctly concatenate incoming and outgoing edges from the underlying directed graph but they're concatenated with the same orientation which produces a set of redundant `EdgeReferences`-- making the adapted graph equivalent to the underlying directed graph as far as the `IntoEdges` implementation is concerned. The `IntoEdges` implementation for undirected graphs produces two `EdgeReference`s for each graph edge-- one for each orientation. This is what allows traversal of edges in either direction. The solution is to make the `UndirectedAdaptor`'s `IntoEdges` behavior match that of an undirected graph by reversing the edges with `Incoming` orientation. That way there will be two `EdgeReferences` for each edge as there are in undirected graphs. <!-- -- Thanks for contributing to `petgraph`! -- -- We require PR titles to follow the Conventional Commits specification, -- https://www.conventionalcommits.org/en/v1.0.0/. This helps us generate -- changelogs and follow semantic versioning. -- -- Start the PR title with one of the following: -- * `feat:` for new features -- * `fix:` for bug fixes -- * `refactor:` for code refactors -- * `docs:` for documentation changes -- * `test:` for test changes -- * `perf:` for performance improvements -- * `revert:` for reverting changes -- * `ci:` for CI/CD changes -- * `chore:` for changes that don't fit in any of the above categories -- The last two categories will not be included in the changelog. -- -- If your PR includes a breaking change, please add a `!` after the type -- and include a `BREAKING CHANGE:` line in the body of the PR describing -- the necessary changes for users to update their code. -- --> --------- Co-authored-by: Paul Jakob Schroeder <paul.schroeder@tangramvision.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 30, 2025
## 🤖 New release * `petgraph`: 0.8.2 -> 0.8.3 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.8.3](https://github.com/petgraph/petgraph/compare/petgraph@v0.8.2...petgraph@v0.8.3) - 2025-09-28 ### Bug Fixes - Infinite `subgraph_isomorphisms_iter` for empty isomorphisms ([#780](#780)) - Algos don't work on `UndirectedAdaptor` ([#870](#870)) ([#871](#871)) - use a queue for SPFA ([#893](#893)) - `StableGraph::reverse` breaks free lists ([#890](#890)) ### Documentation - Fix examples link in README and unify typesetting of one word ([#823](#823)) - Add link to multigraph definition to isomorphism algos ([#824](#824)) - Fix auxiliary space (and time) complexity of bron-kerbosch ([#825](#825)) - Fix Typo in Operator Module Documentation ([#831](#831)) - Sync the crate feature flags in the README and docs ([#832](#832)) - Remove all \[Generic\] tags from algo docstrings ([#835](#835)) - Fix typos in comments ([#836](#836)) - Revamp CONTRIBUTING.md ([#833](#833)) - Update `GraphMap` link in README ([#857](#857)) - Add doc comment for `Dot::with_attr_getters` ([#850](#850)) - Specify iteration order for neighbors and edges and their variants ([#790](#790)) - Collection of Doc fixes ([#856](#856)) ### New Features - Add `into_nodes_edges_iters` to `StableGraph` ([#841](#841)) - Add methods to reserve & shrink `StableGraph` capacity ([#846](#846)) - Add Dinic's Maximum Flow Algorithm ([#739](#739)) - make Csr::from_sorted_edges generic over edge type and properly increase edge_count in Csr::from_sorted_edges ([#861](#861)) - Add `map_owned` and `filter_map_owned` for `Graph` and `StableGraph` ([#863](#863)) - Add dijkstra::with_dynamic_goal ([#855](#855)) - Fix self-loop bug in all_simple_paths and enable multiple targets ([#865](#865)) - mark petgraph::dot::Dot::graph_fmt as public ([#866](#866)) - Add bidirectional Dijkstra algorithm ([#782](#782)) ### Performance - Make A* tie break on lower h-values ([#882](#882)) ### Refactor - add examples for scc algorithms and reorganize into dedicated module ([#830](#830)) - Remove unnecessary trait bounds from impls/methods ([#828](#828)) - replace uses of 'crate::util::zip' with 'core::iter::zip' ([#849](#849)) - Fix clippy (and other) lints ([#851](#851)) - Cleanup repo ([#854](#854)) - replace crate::util::enumerate with Iterator::enumerate ([#881](#881)) ### Testing - Add dependency list for 'quickcheck' feature ([#822](#822)) - Fix feature cfg capitalization in doctest ([#852](#852)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Egor Starovoitov <52821033+starovoid@users.noreply.github.com>
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.
Closes #870
The
IntoEdgesimplementation forUnidirectedAdaptordoes not produce the same set of edges references as an equivalently-constructed undirected graph. This prevents search and traversal algorithms from working if the traversal or search would require traversing an adapted edge whose underlying directed edge is "incoming" (which would prevent traversal in the directed case).The underlying reason is that we currently, correctly concatenate incoming and outgoing edges from the underlying directed graph but they're concatenated with the same orientation which produces a set of redundant
EdgeReferences-- making the adapted graph equivalent to the underlying directed graph as far as theIntoEdgesimplementation is concerned.The
IntoEdgesimplementation for undirected graphs produces twoEdgeReferences for each graph edge-- one for each orientation. This is what allows traversal of edges in either direction.The solution is to make the
UndirectedAdaptor'sIntoEdgesbehavior match that of an undirected graph by reversing the edges withIncomingorientation. That way there will be twoEdgeReferencesfor each edge as there are in undirected graphs.