feature: Approximate State Preparation using Sweeping#183
feature: Approximate State Preparation using Sweeping#183ACE07-Sev wants to merge 6 commits intoamazon-braket:mainfrom
Conversation
- Added `sweeping` module. - Added black box unit test. - Added notebook. - Updated README.md
|
Will I need to fully cover with unit-tests? |
|
@sesmart is this useless now hehe? |
|
Well, objectively very useful as a demonstration and learning resource 😄 - but not eligible for the award 🥉 |
|
It's okay, you'd like me to still work on it? |
|
Hi @ACE07-Sev , thank you for your contribution. Your submission is high quality, and we are happy to continue to work with you to merge it to the main branch. For the purpose of unitaryDESIGN hackathon, our internal review board reviewed all submissions for the corresponding issue (well, two issues), and we decided to award the two bounties to the authors of two different PRs. It was admittedly a close call, and we would have loved to split the bounties. We hope seeing you continue to be active on the Braket repos in the future and getting the opportunity to collaborate. Best regards. |
| def generate_brickwall_ansatz(num_qubits: int, num_layers: int) -> list[UNITARY_LAYER]: | ||
| """Generate a brickwall ansatz with the specified number of qubits and layers. | ||
|
|
||
| Args: |
There was a problem hiding this comment.
The 'brickwall' version of the ansatz doesn't current appear to be covered by testing.
| for i in range(num_layers): | ||
| unitary_layers.append([]) | ||
|
|
||
| for j in range(start_1, num_qubits - 1, 2): | ||
| unitary_layers[i].append(([j, j + 1], np.eye(4, dtype=np.complex128))) | ||
| for j in range(start_2, num_qubits - 1, 2): | ||
| unitary_layers[i].append(([j, j + 1], np.eye(4, dtype=np.complex128))) |
There was a problem hiding this comment.
This definition appears to treat the set of odd-even and even-odd gates as a single 'layer'. This may cause some confusion (as I think it would be more natural to define this as a pair of layers) and may also cause problems for the optimization (since it is probably desirable to optimize the odd-even gates within a row before moving to the even-odd gates, whereas I believe the current setup would alternate between the them). The current definition would also make it impossible use a brickwall circuit with an odd number of rows (i.e. odd-even, even-odd, odd-even gates) which could be optimal for some cases.
| start_2 = 0 | ||
| else: | ||
| start_1 = 0 | ||
| start_2 = 1 |
There was a problem hiding this comment.
Is there a reason for this? I cannot see any obvious reason why this should be necessary.
|
|
||
| @pytest.mark.parametrize("num_qubits", [2, 3, 4, 5]) | ||
| def compile_with_state_sweep_pass(num_qubits: int) -> None: | ||
| state = np.random.uniform(-1, 1, 2**num_qubits) + 1j * np.random.uniform(-1, 1, 2**num_qubits) |
There was a problem hiding this comment.
Its usually not a good idea to use randomly generated states for testing (unless you seed them) as it could cause inconsistent failures.
| compiled_circuit = sweep_state_approximation( | ||
| target_state=state, | ||
| unitary_layers=generate_staircase_ansatz( | ||
| num_qubits=num_qubits, num_layers=int((num_qubits**2) / 2) |
There was a problem hiding this comment.
Can you explain where the value for num_layers comes from? It seems excessively large. From theory I would have thought that num_layers = num_qubits - 2 would be sufficient? I did some quick testing with num_qubits=[3,4,5,6] and confirmed that num_layers=[1,2,3,4] still yields perfect fidelity.
| circuit_tensor_network = self.get_tensor_network_from_unitary_layers( | ||
| num_qubits, unitary_layers |
There was a problem hiding this comment.
I don't know how expensive this is but it seems unnecessary to rebuild the network for every sweep (i.e. rather than just building the network once and mutating the tensors within it).
| num_qubits, unitary_layers | ||
| ) | ||
|
|
||
| if log and i % 20 == 0: |
There was a problem hiding this comment.
It would make sense to have a break here if the results are sufficiently converged (i.e. break if (1-fidelity) < tol).
| """ | ||
| target_mps_adjoint = target_mps.conj() | ||
|
|
||
| current_tn = qtn.MatrixProductState.from_dense( |
There was a problem hiding this comment.
Is there a reason why the state is transformed into an MPS? I would think that this would be a relatively expensive operation (especially when performed every iteration) and I do not see any computational advantage over keeping the state as a dense vector (given that no singular values are truncated in the MPS construction).
4 participants
Issue #, if available:#1194
Description of changes:
Testing done:
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.