Conversation
…em for ppvm. Focused on moving files to more isolated modules to have better appreciation on module responsibility
…Action and FermionSite
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
👋 Thanks for opening your first pull request against PPVM!
A quick note on contribution terms: by submitting this PR you
agree that your contribution is licensed under the
Apache License 2.0
and that you accept the
PPVM Contributor License Agreement.
Please skim those before a maintainer reviews — opening this PR
counts as your acceptance.
A few things that will speed up review:
- Read
CONTRIBUTING.md
for the workflow, build commands, and style notes. - Run
prek run --all-fileslocally; CI runs the same checks. - Use Conventional Commits
for commit messages.
We'll get to your PR as soon as we can. Thanks for contributing!
|
david-pl
left a comment
david-pl
left a comment
There was a problem hiding this comment.
Overall a solid first breakdown of the huge PR, but I think we should make some changes here.
Also, on a more general note: while I get that proving things with lean is beneficial, I think this introduces unneeded complexity at points. For example, to fulfill criteria so this is provably a ring we introduce some traits that are sometimes a bit odd, e.g. Halvable.
cc @Roger-luo
| impl Halvable for f64 { | ||
| #[inline] | ||
| fn half(&self) -> Self { | ||
| *self / 2.0 |
There was a problem hiding this comment.
I'm surprised that this holds up in the exact x.half() + x.half() == x condition.
| /// knows its planes). Operates plane by plane, never scalar on the hot path. | ||
| /// | ||
| /// Design: §"The batch contract". | ||
| pub trait KeyColumn: Default + Clone { |
There was a problem hiding this comment.
Overall, this trait seems a bit complex. Can we split it maybe?
There was a problem hiding this comment.
When split into a package or a separate module?
…of individual bits.
…d structs can be added later if deemed necessary
…anization. So, we now have RotationOne, RotationOneBatch, RotationTwo, and RotationTwoBatch
…operations with respect to the consumer. This allows sepcialized or optimal implementation dependent on the consumer, rather than just inheriting it.
For reference, One use case is symbolic expressions. The minimum requirement of a symbolic expression is that it can be divided by half, and it doesn't matter what number that half comes from. |
| &mut self, | ||
| qubit0: usize, | ||
| qubit1: usize, | ||
| p: [C; 3], |
There was a problem hiding this comment.
This might need similar treatment, like the pauli channrl error coefficient factors, too, because we assume the three parameters also satisfy the normalization condition, right?
| } | ||
|
|
||
| /// Batched Clifford gates: apply the same gate to many qubits in one call. | ||
| pub trait CliffordBatch: Clifford { |
There was a problem hiding this comment.
I was thinking if there is a way to generalize this based on the Clifford trait, but I didn't really find a good way to define a broadcast trait in general, so I ended up doing this.
3 participants
A patch to move the changes to ppvm-trait from roger's PR #204
Additionally, the following changes were made: