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

Add BasePass.get_pre_conditions and BasePass.get_post_conditions #1689

Merged
merged 19 commits into from
Nov 25, 2024

Conversation

sjdilkes
Copy link
Contributor

@sjdilkes sjdilkes commented Nov 21, 2024

Adds two new methods to BasePass in passes.cpp to expose pre conditions and post conditions for a pass to the python layer as List[Predicate].

Related issues

#1675

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@sjdilkes sjdilkes marked this pull request as ready for review November 21, 2024 13:28
@sjdilkes sjdilkes requested a review from cqc-alec November 21, 2024 13:28
pytket/binders/passes.cpp Outdated Show resolved Hide resolved
}
return pre_conditions;
},
"Returns the pre condition Predicates for the given pass."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Returns the pre condition Predicates for the given pass."
"Returns the precondition Predicates for the given pass."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"Returns the pre condition Predicates for the given pass."
"\n:return: A list of Predicate")
.def(
"get_post_conditions",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"get_post_conditions",
"get_postconditions",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}
return post_conditions;
},
"Returns the post condition Predicates for the given pass."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Returns the post condition Predicates for the given pass."
"Returns the postcondition Predicates for the given pass."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

pytket/binders/passes.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is also worth adding a function specifically to get the gate-set requirements of a pass; i.e. which gets the preconditions, checks for any GateSetPredicates, and returns the set intersection of their gate sets (or None if there are no GateSetPredicates.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -23,7 +23,7 @@

class TketConan(ConanFile):
name = "tket"
version = "1.3.48"
version = "1.3.49"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This version bump shouldn't be necessary as the change only touches pytket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops ofc - will revert

sjdilkes and others added 6 commits November 21, 2024 14:12
Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>
Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>
@sjdilkes sjdilkes requested a review from cqc-alec November 22, 2024 11:26
OpTypeSet allowed_ops;
for (const std::pair<const std::type_index, std::shared_ptr<tket::Predicate>>
&p : base_pass.get_conditions().first) {
if (p.second->to_string().substr(0, 17) == "GateSetPredicate:") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this check is necessary. We can just use dynamic_cast and ignore it if it returns nullptr.

if (p.second->to_string().substr(0, 17) == "GateSetPredicate:") {
std::shared_ptr<GateSetPredicate> gsp_ptr =
std::dynamic_pointer_cast<GateSetPredicate>(p.second);
if (allowed_ops.empty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will end up giving the wrong result in the (admittedly unlikely) case that we have already encountered two GateSetPredicates with empty intersection. I suggest using a std::optional<OpTypeSet>, where no-value means all all ops are allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done (I think)

Comment on lines 323 to 325
"Returns the intersection of all set of OpType for all "
"GateSetPredicate in the `BasePass` preconditions.",
"\n\n:return: A set of allowed OpType")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Returns the intersection of all set of OpType for all "
"GateSetPredicate in the `BasePass` preconditions.",
"\n\n:return: A set of allowed OpType")
"Returns the intersection of all set of OpType for all "
"GateSetPredicate in the `BasePass` preconditions, or `None` if there are no gate-set predicares.",
"\n\n:return: A set of allowed OpType, or `None`")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -38,7 +38,7 @@ def requirements(self):
self.requires("pybind11_json/0.2.14")
self.requires("symengine/0.13.0")
self.requires("tkassert/0.3.4@tket/stable")
self.requires("tket/1.3.49@tket/stable")
self.requires("tket/1.3.50@tket/stable")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't need updating.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reverted - let's see if the same issue comes up

Comment on lines 1110 to 1111
assert OpType.CX in gate_set # type: ignore
assert OpType.Measure in gate_set # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be able to avoid the type: ignores by first asserting that gate_set is not None.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -23,7 +23,7 @@

class TketConan(ConanFile):
name = "tket"
version = "1.3.49"
version = "1.3.50"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't be needed.

@sjdilkes sjdilkes requested a review from cqc-alec November 25, 2024 11:58
@sjdilkes sjdilkes merged commit ba0f7c0 into main Nov 25, 2024
30 checks passed
@sjdilkes sjdilkes deleted the gateset-from-GateSetPredicate branch November 25, 2024 12:23
CalMacCQ pushed a commit that referenced this pull request Nov 29, 2024
…1689)

* Add new methods for getting predicates from python passes

* bump

* Update predicates_test.py

* Update pytket/binders/passes.cpp

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>

* Update pytket/binders/passes.cpp

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>

* add get gate_set, check tests

* Update predicates_test.py

* Update predicates_test.py

* Update predicates_test.py

* bump, fix ruff issue

* regen stubs

* Update predicates_test.py

* Update predicates_test.py

* Update predicates_test.py

* addresss comments

* Update predicates_test.py

---------

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>
CalMacCQ pushed a commit that referenced this pull request Nov 29, 2024
…1689)

* Add new methods for getting predicates from python passes

* bump

* Update predicates_test.py

* Update pytket/binders/passes.cpp

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>

* Update pytket/binders/passes.cpp

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>

* add get gate_set, check tests

* Update predicates_test.py

* Update predicates_test.py

* Update predicates_test.py

* bump, fix ruff issue

* regen stubs

* Update predicates_test.py

* Update predicates_test.py

* Update predicates_test.py

* addresss comments

* Update predicates_test.py

---------

Co-authored-by: Alec Edgington <54802828+cqc-alec@users.noreply.github.com>
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