Skip to content
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

Allow TopologicalSorter without error checking #1160

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

jakelishman
Copy link
Member

@jakelishman jakelishman commented Apr 5, 2024

For representative examples in Qiskit, this reduces the runtime of interacting with the topological sorter by around 15-20%.

Built on top of #1159. This is the dynamic-check version of #1158.

Close #1158.


Using the same timings as #1159, but adding check_args=False, the same test drops to 233µs for me, which is another 15% (though I've seen more in other graphs). There is maybe a 1% hit for the new Option checking of node2state in the check_args=True case, but that might just have been statistical noise on my machine.

This optimises the creation of the `list` objects returned from
`TopologicalSorter.get_ready` (by avoiding a temporary `Vec`) and allows
`done` to accept a single index, avoiding the need for the user to
allocate many small lists in several real-world uses.
For representative examples in Qiskit, this reduces the runtime of
interacting with the topological sorter by around 15-20%.
@coveralls
Copy link

coveralls commented Apr 5, 2024

Pull Request Test Coverage Report for Build 8639062108

Details

  • 24 of 25 (96.0%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 96.494%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/toposort.rs 24 25 96.0%
Files with Coverage Reduction New Missed Lines %
src/toposort.rs 1 90.91%
src/shortest_path/all_pairs_bellman_ford.rs 6 95.53%
Totals Coverage Status
Change from base Build 8638957457: -0.01%
Covered Lines: 17314
Relevant Lines: 17943

💛 - Coveralls

Copy link
Contributor

@enavarro51 enavarro51 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@enavarro51 enavarro51 merged commit e7b0d6f into Qiskit:main Apr 10, 2024
28 checks passed
@jakelishman jakelishman deleted the toposort-check-args branch April 11, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version of TopologicalSorter without error checking
3 participants