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

feat: Simpler convex checker, no longer requires &mut #114

Merged
merged 3 commits into from
Oct 20, 2023

Conversation

aborgna-q
Copy link
Collaborator

@aborgna-q aborgna-q commented Oct 20, 2023

ConvexChecker now uses the pre-computed node toposort to compute traverse the subgraph nodes and their causal future in order.

This should end up processing less nodes than the previous version, and let us avoid the temporary datastructure that required &mut checker all throughout the API.

I added some simple benchmarks, but unfortunately they are not the best at showing the improvements in this refactor (we end up traversing most of the nodes anyway).
In more real tests on tket2 I saw small improvements, but nothing to write home about.

By using the toposorted order and keeping track
of nodes in the causal future.
@aborgna-q aborgna-q requested a review from ss2165 October 20, 2023 08:02
Copy link
Member

@ss2165 ss2165 left a comment

Choose a reason for hiding this comment

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

The code simplification is good to have even if performance improvements aren't major

@aborgna-q aborgna-q added this pull request to the merge queue Oct 20, 2023
Merged via the queue into main with commit f743b4c Oct 20, 2023
@aborgna-q aborgna-q deleted the perf/convex-check branch October 20, 2023 10:00
github-merge-queue bot pushed a commit that referenced this pull request Oct 20, 2023
## 🤖 New release
* `portgraph`: 0.9.0 -> 0.10.0 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## 0.10.0 (2023-10-20)

### Features

- Simpler convex checker, no longer requires `&mut`
([#114](#114))

### Miscellaneous Tasks

- [**breaking**] Update pyo3 requirement from 0.19 to 0.20
([#110](#110))

### Doc

- Add DEVELOPMENT.md
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

---------

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Agustín Borgna <[email protected]>
@github-actions github-actions bot mentioned this pull request Oct 31, 2023
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.

2 participants