Skip to content
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 get_tensor_network_state method #5

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

NGBigField
Copy link
Contributor

In response to issue #4:

Main changes:

  • Some attributes of SimpleUpdate class are now properties, using the @property decorator.
  • Moved methods that absorb weights into TensorNetwork class. This supports the following point.
  • Added the method get_tensor_network_state() which returns a copy of the TensorNetwork object, with weights absorbed into neighboring tensors, to reflect the actual state of the system
  • In tnsu/examples.py in afh_peps_ground_state_experiment() instead of saving the original TensorNetwork, now the return value of get_tensor_network_state() is used
  • Some type-hints were added in various places in Both TensorNetwork and SimpleUpdate classes. Mostly to annotate return values to allow linters to provide coding-hints for users, according to PEP 484.

Possible disagreements or issues:

The return type of get_tensor_network_state() is currently a TensorNetwork object.
It might make more sense to return a simpler list of tensors (i.e., type list[np.ndarray]), where the list is simply self.tensors.

At the moment, this changes nothing.
It will, in the future, to remember that those attributes actually belong to TensorNetwork and not to SimpleUpdate.

If someone changes them from the TensorNetwork attribute, these will also change.
The current structure reflects this fact.
Moved methods that absorb weights into TensorNetwork class.

Created `TensorNetwork .get_tensor_network_state()` method
@RoyElkabetz
Copy link
Owner

Thank you very much for putting this together, it looks very good.
I have a few comments/questions which you can see below.

Copy link
Owner

@RoyElkabetz RoyElkabetz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good, thank you. I have left a few comments.

src/tnsu/tensor_network.py Outdated Show resolved Hide resolved
src/tnsu/tensor_network.py Show resolved Hide resolved
src/tnsu/simple_update.py Show resolved Hide resolved
@NGBigField
Copy link
Contributor Author

I've responded to the comments above. Tell me if you wish to change something.

@RoyElkabetz
Copy link
Owner

It all looks very good. Just a final question, did you had the chance executing the test module, just to check that everything still runs properly? If not, could you do that? just run test_main.py.

+ run all tests in `test_main`
@NGBigField
Copy link
Contributor Author

It all looks very good. Just a final question, did you had the chance executing the test module, just to check that everything still runs properly? If not, could you do that? just run test_main.py.

I pass all tests besides test_triangle_lattice(). To be fair, branch "main" also fails this test.

@RoyElkabetz
Copy link
Owner

I pass all tests besides test_triangle_lattice(). To be fair, branch "main" also fails this test.

Good enough!

@RoyElkabetz RoyElkabetz merged commit cc0542d into RoyElkabetz:main Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants