Conversation
Both modified physical_systems.
…erStateto be consistent with current PhysicalSystem definitions.
kochkov92
left a comment
kochkov92
left a comment
There was a problem hiding this comment.
apparently didn't send this initial set of comments - if this is blocking, I think we can check this in assuming that you've tested it enough and revisit in a follow-up PR, up to you.
| """ | ||
| neel_probabilities = jnp.asarray(neel_probabilities) | ||
| sample_key, basis_key = jax.random.split(key, 2) | ||
| basis_val_start = jax.random.choice(basis_key, np.arange(2), |
There was a problem hiding this comment.
I forgot - is np.arange(2) x and z? I though that 0, 1, 2 were mapped to x, y z basis.
There was a problem hiding this comment.
I think overall you probably want:
xz_basis = ...
zx_basis = ...
basis = jax.random.choice(basis_key, np.stack([xz_basis, zx_basis]))
|
|
||
|
|
||
| class ClusterState(PhysicalSystem): | ||
| """Implementation for cluster state hamiltonian. |
There was a problem hiding this comment.
nit: Add one blank like separating one-line comment and verbose comment.
| return expanded_lattice | ||
|
|
||
| def _get_stabilizer_groups(self) -> list[node_collections.NodesCollection]: | ||
| """Constructs `NodeCollection`s for all groups in cluster state Hamiltonian. |
There was a problem hiding this comment.
super nit: make it 1-line or a full docstring
There was a problem hiding this comment.
This is essentially one-line doc, you are saying the """ should be in the same line?
I was looking at quimb doc and it looks like they do this in general
"""Some doc
"""
| def _get_stabilizer_groups(self) -> list[node_collections.NodesCollection]: | ||
| """Constructs `NodeCollection`s for all groups in cluster state Hamiltonian. | ||
| """ | ||
| expanded_lattice = self._get_expanded_lattice() |
There was a problem hiding this comment.
do you want to add self._lattice similar to the previous instance?
| lengths = set( | ||
| [len(stabilizer_bond) for stabilizer_bond in stabilizer_bonds] | ||
| ) | ||
| stabilizer_bonds_groups = [] |
There was a problem hiding this comment.
as per offline chat flagging that this is being reset inside of the loop and used outside later. This is probably still correct because you accumulate stabilizer_bonds that is outside of the loop, but might be worthwhile trying to refactor these steps to be a bit more constructive.
| ] | ||
| return all_terms | ||
|
|
||
| def get_ham(self) -> qtn.MatrixProductOperator: |
There was a problem hiding this comment.
can use the default value here (since it's the same)
| stabilizer_nodes_groups.append(stabilizer_nodes) | ||
| return stabilizer_nodes_groups | ||
|
|
||
| def _get_all_terms_groups(self) -> list[types.TermsTuple]: |
There was a problem hiding this comment.
let's discuss some of these construction methods in more details to see if we can build something a bit easier to parse.
2 participants
Added ClusterState implementation in physical_systems.py and its basis sampler in mps_sampling;
I accidentally trimmed white space, so it resulted in a lot of visual changes, sorry...