Skip to content

feat(quil-py): Add an immutable FrozenProgram class #384

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MarquessV
Copy link
Contributor

This is a proposal for #251

The API is very simple, and only supports serialization at the moment.

Copy link

github-actions bot commented Jul 31, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://rigetti.github.io/quil-rs/pr-preview/pr-384/
on branch quil-py-docs at 2024-07-31 22:47 UTC

pub struct FrozenProgram {
program: PyProgram,
}
impl_eq!(FrozenProgram);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question: The __richcmp__ implementation this macro generates will only compare two FrozenPrograms with equivalent inner Programs as equal.

What about the case where a FrozenProgram is compared to a Program. If the inner program of a FrozenProgram instance is equivalent to an instance of Program, should they evaluate as equivalent?

Choose a reason for hiding this comment

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

I'm flexible. If converting a Program to a FrozenProgram is a light operation (it encapsulates the Program and exposes a limited immutable interface but does not deeply copy the program) then at the level of quil-py I would adhere to its Rust heritage and force the user to write "if frozen_program == FrozenProgram(program):".

How this is net exposed in pyquil is a separate question.

Choose a reason for hiding this comment

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

Strike that ill-informed comment of mine.

I can see it takes the clone. Of course it needs to as the user has a mutable reference to it already.

Let me think about the performance implications of typical workflows here and get back to you.

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