-
Notifications
You must be signed in to change notification settings - Fork 12
Feature/hamiltonian generation #22
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
Feature/hamiltonian generation #22
Conversation
Codecov Report❌ Patch coverage is
... and 11 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| """ | ||
| Test properties of a larger lattice (2x2 square) without full matrix comparison. | ||
| """ | ||
| lattice = SquareLattice(size=(2, 2), pbc=True) # 4 sites, 8 bonds with PBC |
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.
2*2 with PBC is subtle, as you can see, there will be two bonds between the same two lattice points
|
over all looks good, a nice work, only some minor details require fix or explanantion |
This commit refactors the hamiltonian generation functions to be more robust, flexible, and aligned with the project's backend-agnostic architecture, based on feedback from the recent code review. Key changes include: - **Backend Agnosticism:** The `PauliStringSum2COO` call now uses `numpy=False`, ensuring the returned sparse matrix is a native tensor of the currently active backend (TensorFlow, JAX, etc.). The return type hints have been updated to `Any` to reflect this. - **Anisotropic Heisenberg Model:** The `heisenberg_hamiltonian` function now accepts a list or tuple for `j_coupling`, allowing for the definition of anisotropic models with different Jx, Jy, and Jz values. - **Test Suite Enhancement:** Tests have been updated to use `tc.backend.to_dense` for backend-agnostic validation. A new test case for the anisotropic Heisenberg model has also been added to ensure its correctness. - **Code Cleanup:** A redundant `float()` cast was removed from the Rydberg hamiltonian logic.
| from typing import Any, List, Tuple, Union | ||
| import numpy as np | ||
| from tensorcircuit.cons import dtypestr, backend | ||
| import tensorcircuit as tc |
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.
dont directly use tensorcircuit internally, use from .cons import dtypestr, backend and use backend.blah in the code instead of tc.backend
Internal API Usage: I've switched to importing backend and dtypestr from .cons and PauliStringSum2COO from ..quantum as you suggested, avoiding direct calls to the top-level tc package. Error Handling: For both heisenberg_hamiltonian and rydberg_hamiltonian, I've added a check to raise a ValueError if num_sites is 0, which is indeed more explicit.
|
Hi @refraction-ray, Internal API Usage: I've updated the code to use backend from .cons and relative imports for internal modules, instead of calling the top-level tensorcircuit package. Error Handling: I've added a ValueError check for num_sites = 0 in both Hamiltonian functions, as you suggested. Rydberg Hamiltonian Sign: Regarding your question on the Z-term sign (why -v_ij/4?), I've double-checked the derivation and I thought the implementation is correct. The single-body Z coefficient comes from two sources: To make this clearer for future reference, I've also added a comment inside the function explaining this derivation. I believe this addresses all your feedback. The PR should be ready for another look. Thanks again |
| import typing | ||
| from typing import Any, List, Tuple, Union | ||
| import numpy as np | ||
| from tensorcircuit.cons import dtypestr, backend |
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.
should also be from ..cons ?
| @@ -0,0 +1,154 @@ | |||
| import typing | |||
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.
should merge with the following line? import all types you will use
This commit addresses code review feedback on the new hamiltonians module. - Switched from absolute to relative imports for internal modules (`..cons`, `..quantum`) to improve modularity. - Cleaned up and unified `typing` imports for better code style and consistency.
|
Thanks for the review, @refraction-ray! I've updated the PR according to your suggestions. It should be ready for another look now. |
|
great! LGTM. will merge if CI finished successfully. |
This pull request implements Task 1 of the follow-up plan, introducing utility functions to generate common spin Hamiltonians from
Latticeobjects.Key Contributions:
generate_heisenberg_hamiltonian: Creates a sparse Heisenberg Hamiltonian based on the nearest-neighbor pairs provided by the lattice.generate_rydberg_hamiltonian: Creates a sparse Rydberg Hamiltonian, utilizing the lattice's distance matrix to calculate long-range interactions.tests/templates/test_hamiltonians.py) with unit tests that verify: