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.

Sign up for GitHub

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

Jump to bottom

Improve Timing Graph #61

Merged
merged 65 commits into from
Jun 1, 2022
Merged

Improve Timing Graph #61

merged 65 commits into from
Jun 1, 2022

Conversation

kalzoo
Copy link
Contributor

@kalzoo kalzoo commented Apr 5, 2022 •
edited
Loading

Closes #64

TODO:

  • Consider hiding this API behind a feature, such as dot-graph? It's already a breaking change so that shouldn't be much trouble

Fix to Timing Dependency Bug

By way of example, here's the old graph and the new one, for the same program. Note that the second one has fewer memory dependencies.

Broken

graphviz (1)

Fixed

graphviz

Fix to GraphViz API

This PR replaces the brittle templating currently in place with the use of the dot_writer crate so as to offload that logic.

kalzoo and others added 30 commits March 7, 2022 08:52
@kalzoo
New: Expression#substitute_variables
956967b
@kalzoo
New: ProgramError
20cdefb
@kalzoo
Update: parse DEFCAL MEASURE
96c1228
@kalzoo
New: Instruction#apply_to_expressions
be3e7e5
@kalzoo
Fix: parse DEFCAL MEASURE
abf3692
@kalzoo
Update: add DEFCAL MEASURE expansion
d2a7204
@kalzoo
Fix: tolerate program composition instructions in ScheduledProgram#build
d18e5fd
@kalzoo
Fix: consider PRAGMA a classical compute instruction
fed814b
@kalzoo
Chore: add thiserror
0bdd847
@kalzoo
Chore: fmt
44fce8c
@kalzoo
Chore: lint
165c4a7
@kalzoo
Chore: improve docs
fbc7afd
@kalzoo
Decompose parse_defcal function
d0e4448
@kalzoo
Improve type annotation for apply_to_expression
e090669
@kalzoo
Chore: improve inline docs
7a7eeb6
@kalzoo
Fix: PRAGMA display
3c7ff00
@kalzoo
Fix: DEFCAL MEASURE parsing
bdc4c13
@kalzoo
Improve DEFCAL MEASURE expansion readability
14ce2d1
@kalzoo
Fix: construct CalibrationSet::default()
9e99f00
@kalzoo
Chore: fmt
653d0b2
@kalzoo
Merge branch 'main' into 43-program-calibration-ux
ee16e7e
@kalzoo
Merge branch 'main' into 43-program-calibration-ux
0929ef3
@semantic-release-bot @kalzoo
Release v0.8.4 [skip ci]
8de8287
@nilslice @kalzoo
chore: include latest Cargo.lock
472ac22
@nilslice @kalzoo
feat: update program to use btreemap for deterministic ordering
3eda533
@nilslice @kalzoo
fix: remove unused imports
1fbda96
@nilslice @kalzoo
chore: add update check to cargo.lock in CI, include in assets list
75a47ca
@nilslice @kalzoo
chore: use single string input, revert from slice to check indicies
9a2beff
@nilslice @kalzoo
fix: move cargo update inline with semantic-release prepare cmd
91140a5
@nilslice @kalzoo
Update: update Cargo.lock version (#52)
5e15122
@kalzoo kalzoo changed the title Improve Dot Graph Improve Timing Graph May 3, 2022
@kalzoo kalzoo requested review from dbanty and nilslice and removed request for dbanty May 3, 2022 03:07
@dbanty
Copy link
Contributor

dbanty commented May 3, 2022

+1 to hiding behind a feature to reduce downstream dependency leak.

dbanty
dbanty reviewed May 3, 2022
View reviewed changes
Copy link
Contributor

@dbanty dbanty left a comment

Choose a reason for hiding this comment

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

I don't know much about the timing graph in particular, but I did leave some Rusty suggestions

src/program/dot.rs Outdated Show resolved Hide resolved
src/program/dot.rs Outdated Show resolved Hide resolved
Base automatically changed from 43-program-calibration-ux to main May 31, 2022 23:01
genos
genos reviewed Jun 1, 2022
View reviewed changes
Copy link
Contributor

@genos genos left a comment

Choose a reason for hiding this comment

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

Nicely done! One note re: consistency of copyright docstring, but (once this passes CI) LGTM

src/lib.rs Show resolved Hide resolved
src/program/graphviz_dot.rs Outdated Show resolved Hide resolved
@kalzoo kalzoo merged commit e10749c into main Jun 1, 2022
@github-actions
Copy link

github-actions bot commented Jun 1, 2022

🎉 This PR is included in version 0.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbanty dbanty dbanty left review comments

@genos genos genos left review comments

@nilslice nilslice Awaiting requested review from nilslice

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve Timing Graph
6 participants
@kalzoo @dbanty @genos @semantic-release-bot @nilslice @notmgsk