-
Notifications
You must be signed in to change notification settings - Fork 62
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 binary_encoder
parameterized in hyperspherical coordinates
#1584
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1584 +/- ##
=======================================
Coverage 99.06% 99.06%
=======================================
Files 76 76
Lines 11322 11387 +65
=======================================
+ Hits 11216 11281 +65
Misses 106 106
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Run on QPU
|
Co-authored-by: Alejandro Sopena <44305203+AlejandroSopena@users.noreply.github.com>
Co-authored-by: Alejandro Sopena <44305203+AlejandroSopena@users.noreply.github.com>
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.
Thanks, just some minor comments.
@@ -668,6 +674,8 @@ def _generate_rbs_angles(data, nqubits: int, architecture: str): | |||
r_array[j - 1] = math.sqrt(r_array[2 * j] ** 2 + r_array[2 * j - 1] ** 2) | |||
phases[j - 1] = math.acos(r_array[2 * j - 1] / r_array[j - 1]) | |||
|
|||
phases = np.array([float(phase) for phase in phases]) |
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.
is there any specific reason why the phases are not returned as backend specific arrays? (thus with the usual backend.cast)
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.
all functions in this module are backend-agnostic as of right now. If we were to introduce the backend
as an argument, I'd prefer to do it in all encodings are once on a separate PR.
for weight in range(1, nqubits): | ||
n_choose_k = int(binom(nqubits, weight)) | ||
cummul_n_k += n_choose_k | ||
placeholder = np.random.rand(n_choose_k) |
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.
not sure how big n_choose_k
can get, but just as a reminder generating samples with numba and cupy should be faster if it's big.
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.
The size is the regular binomial coefficient, tending to
|
||
cummul_n_k = 0 | ||
initial_string = np.array([1] + [0] * (nqubits - 1)) | ||
for weight in range(1, nqubits): |
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.
This is a very big loop, it might be clearer to make it as separate function:
for weight in range(1, nqubits):
separate_function(...)
Co-authored-by: BrunoLiegiBastonLiegi <45011234+BrunoLiegiBastonLiegi@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: BrunoLiegiBastonLiegi <45011234+BrunoLiegiBastonLiegi@users.noreply.github.com>
This is based on Section V of https://arxiv.org/abs/2405.20408 and it works for real- and complex-valued data.
Checklist: