Add additional lattice models #6237
Conversation
pennylane/spin/lattice.py
Outdated
| vectors = math.eye(3) | ||
| positions = [[0, 0, 0], [0.5, 0.5, 0.5]] | ||
|
|
||
| n_cells = n_cells[0:3] |
There was a problem hiding this comment.
Not sure I agree with lines like these. If a user passes in n_cells=[1, 1, 2, 2, 3, 3...] they don't get an error. It would just truncate it down to the first 3 values, right? To me it should raise a friendly error rather than silently truncate it.
There was a problem hiding this comment.
But I'm aware this has already been merged for the other internal functions so 🤷
There was a problem hiding this comment.
How about we raise an error, if anything other than 1 is passed in the rest of the dimensions?
There was a problem hiding this comment.
What purpose does having n_cells longer than the intended shape serve? If nothing, we should err on the side of raising an error/warning about skipping the rest of the values.
There was a problem hiding this comment.
It was to provide flexibility to the user. I think we can raise a warning, or raise an error if anything in the higher dimension is other than 0 or 1. Please let me know if you have a preference for one over the other.
ddhawan11
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6237 +/- ##
==========================================
- Coverage 99.58% 99.58% -0.01%
==========================================
Files 443 443
Lines 42273 42227 -46
==========================================
- Hits 42096 42050 -46
Misses 177 177 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks @ddhawan11, looks good overall. We just need stronger tests for the correctness of the lattices.
|
|
||
| @pytest.mark.parametrize( | ||
| # expected_points here were calculated manually. | ||
| ("shape", "n_cells", "expected_points"), |
There was a problem hiding this comment.
Do we have such tests for all lattices or just for 3D ones?
There was a problem hiding this comment.
yes, we test the rest of the cases when we test the lattice class.
pennylane/spin/lattice.py
Outdated
| vectors = math.eye(3) | ||
| positions = [[0, 0, 0], [0.5, 0.5, 0.5]] | ||
|
|
||
| n_cells = n_cells[0:3] |
There was a problem hiding this comment.
What purpose does having n_cells longer than the intended shape serve? If nothing, we should err on the side of raising an error/warning about skipping the rest of the values.
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Context:
Adds additional lattice templates for the lattice class.
Description of the Change:
Adds lattice models: Lieb, Cubic, Body centered cubic and Face centered cubic.
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-70987]