-
Notifications
You must be signed in to change notification settings - Fork 54
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
Hamming weight phasing with configurable number of ancilla #1450
base: main
Are you sure you want to change the base?
Conversation
…e, still debugging.
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Thanks for opening a PR! Please ping me or @tanujkhattar when this is ready for review or if you have any specific questions |
#print("shape after flatten: ", np.shape(x_parts)) | ||
for part in x: | ||
print("next elem: ", part) | ||
x = bb.join(x_parts, dtype=QUInt(self.bitsize.bit_length())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try bb.join(np.array(x_parts), ...)
Elsewhere, we tried to be careful about accepting either lists or numpy arrays but seem to have missed this one.
#1470
…bloq(), added test_hamming_weight_phasing_with_configurable_ancilla().
@mpharrigan This should be in a functional state at least. There are a few TODO comments which indicate things I couldn't immediately tell how to do but aren't essential, so suggestions for those would be appreciated. I also have a few questions:
All feedback is welcome, especially on the tests as I do not have much experience with pytest. I expect to have to make quite a few modifications to this. |
Hi @dobbse42! Here is some feedback on the PR, hope it is helpful!
You can use the
The current standard way is to use Qualtran/qualtran/resource_counting/_bloq_counts_test.py Lines 82 to 85 in 7bbec3b
If you only care about the T-cost, you can use the .total_t_count method to get it (see class GateCounts for more such helper functions)
We usually pick a few random small ones (especially for simulation tests), and a few slightly larger sizes (e.g. for gate cost tests). If you feel there are some edge cases that require careful testing, it's good to add more tests in!
If the paper only has upper bounds, you can add "less than equal" tests against the reference formulas. Sometimes the formulas may only be asymptotically valid, or be missing constant factors, in which case you can leave out precise gate count tests. Another useful contribution is to plot the T-costs vs input size in the bloq notebook, just to visually verify.
The override is needed when the bloq cannot be decomposed (e.g. because of symbolic bitsizes). It is also good to add the override if the call graph is much simpler (and efficient to compute) compared to the bloq decomposition. E.g. in this particular case, the decomposition requires a lot of splits and joins, but the call graph is much simpler. So I'd say keep the override for convenience. In case the bloq has a valid decomposition but the call graph still gives an error, that is a bug, please report it! |
# TODO: Surely there is a better way of doing this | ||
if remainder > 1: | ||
|
||
return { | ||
HammingWeightPhasing(self.ancillasize+1, self.exponent, self.eps): num_iters, | ||
HammingWeightPhasing(remainder, self.exponent, self.eps): bool(remainder), | ||
} | ||
elif remainder: | ||
return { | ||
HammingWeightPhasing(self.ancillasize+1, self.exponent, self.eps): num_iters, | ||
ZPowGate(exponent=self.exponent, eps=self.eps): 1 | ||
} | ||
else: | ||
return { | ||
HammingWeightPhasing(self.ancillasize+1, self.exponent, self.eps): num_iters, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use a Counter
:
counts = Counter[Bloq]()
counts[HammingWeightPhasing(self.ancillasize+1, self.exponent, self.eps)] += num_iters
if remainder > 1:
counts[HWP(remainder, ...)] += 1
elif remainder:
counts[ZPowGate] += 1
return counts
…le_ancilla.build_call_graph()
#645: Added HammingWeightPhasingWithConfigurableAncilla and a temp test file, still debugging. TODO: add unit tests to hamming_weight_phasing_test.py
Currently having issues joining the slices of the phased register back together with
x = bb.join(x_parts, dtype=QUInt(self.bitsize.bit_length()))
at the end of build_composite_bloq() (line 272).