Skip to content

Commit

Permalink
Fix sparse grid tests
Browse files Browse the repository at this point in the history
  • Loading branch information
pfebrer committed Oct 10, 2024
1 parent 0cc03dc commit a12800f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 23 deletions.
13 changes: 13 additions & 0 deletions src/sisl/_sparse_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,19 @@ def reduce_orbital_products(
The output grid. If ``out`` is not ``None``, it will be the same as ``out``. Otherwise
it will be a newly created array.
"""
if weights.shape[0] != self.geometry.no:
raise ValueError(
f"Mismatch in weights array shape: Number of unit cell orbitals is {weights.shape[0]}, while orbital values are stored for {self.geometry.no} orbitals."
)
elif weights.shape[1] != self.geometry.no_s:
raise ValueError(
f"Mismatch in weights array shape: The number of unit cell orbitals ({self.geometry.no})"
f" is correct, but supercell orbitals in the weights array is {weights.shape[1]},"
f" while it should be {self.geometry.no_s}. It is likely that the weights array"
f" has been obtained from a matrix with the wrong number of auxiliary cells."
f" The correct number of auxiliary cells is {self.geometry.nsc}."
)

csr = self._csr

# Find out the reduced shape, and the reduce factor. The reduced factor is the number
Expand Down
23 changes: 13 additions & 10 deletions src/sisl/physics/densitymatrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ def density(
spinor=None,
atol: float = 1e-7,
eta: Optional[bool] = False,
method: Literal["pre-compute", "direct"] = "direct",
method: Literal["pre-compute", "direct"] = "pre-compute",
**kwargs,
):
r"""Expand the density matrix to the charge density on a grid
Expand Down Expand Up @@ -758,9 +758,8 @@ def density(
show a progressbar on stdout
method:
It determines if the orbital values are computed on the fly (direct) or they are all pre-computed
on the grid at the beginning(pre-compute).
on the grid at the beginning (pre-compute).
Pre computing orbitals results in a faster computation, but it requires more memory.
Currently pre-computing has a bug that results in out-of-bounds, it will be fixed.
Notes
-----
Expand All @@ -772,6 +771,16 @@ def density(
# same result.
uc_dm = self.translate2uc()

if method == "pre-compute":
# Compute orbital values on the grid
psi_values = uc_dm.geometry._orbital_values(grid.shape)

# Here we just set the nsc to whatever the psi values have.
# If the nsc is bigger in the DM, then some elements of the DM will be discarded.
# If the nsc is smaller in the DM, then the DM is just "padded" with 0s.
if not np.all(uc_dm.nsc == psi_values.geometry.nsc):
uc_dm.set_nsc(psi_values.geometry.nsc)

# Get the DM components with which we want to compute the density
csr = uc_dm._csr
if self.spin.kind > Spin.POLARIZED:
Expand Down Expand Up @@ -806,7 +815,7 @@ def density(
# Create the DM csr matrix.
csrDM = csr_matrix(
(DM, csr.col[idx], _ncol_to_indptr(csr.ncol)),
shape=(self.shape[:2]),
shape=(uc_dm.shape[:2]),
dtype=DM.dtype,
)

Expand Down Expand Up @@ -836,13 +845,7 @@ def density(
csrDM = csr.tocsr(dim=0)

if method == "pre-compute":
raise NotImplementedError(
"Currently a memory bug is happening, cannot be used until fixed, use the direct method"
)
try:
# Compute orbital values on the grid
psi_values = uc_dm.geometry._orbital_values(grid.shape)

psi_values.reduce_orbital_products(
csrDM, uc_dm.lattice, out=grid.grid, **kwargs
)
Expand Down
10 changes: 1 addition & 9 deletions src/sisl/physics/tests/test_density_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,7 @@ def func(D, ia, atoms, atoms_xyz):

@pytest.fixture(
scope="module",
params=[
"direct",
pytest.param(
"pre-compute",
marks=pytest.mark.xfail(
reason="raises NotImplementedError due to seg-fault"
),
),
],
params=["direct", "pre-compute"],
)
def density_method(request):
return request.param
Expand Down
5 changes: 1 addition & 4 deletions src/sisl/tests/test_sparse_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def geometry():

orb = sisl.AtomicOrbital("2pzZ", (r, f))
geom = sisl.geom.graphene(orthogonal=True, atoms=sisl.Atom(6, orb))
geom.set_nsc([3, 5, 1])

return geom

Expand Down Expand Up @@ -92,7 +93,6 @@ def test_wavefunction_correct(H, grid_shape, psi_values, k):
assert np.allclose(from_psi_grid.grid, wf_grid.grid)


@pytest.mark.skip(reason="bug in array indices, data-sc-off (sometimes)")
@pytest.mark.parametrize(
["k", "ncoeffs"],
[[(0, 0, 0), 1], [(0, 0, 0), 2], [(0.25, 0, 0), 1], [(0.25, 0, 0), 2]],
Expand Down Expand Up @@ -149,7 +149,6 @@ def test_onthefly_reduction(geometry, psi_values, k, ncoeffs):
assert np.allclose(reduced.reshape(post_reduced.shape), post_reduced)


@pytest.mark.skip(reason="bug in array indices, data-sc-off (sometimes)")
def test_orbital_products(geometry):
"""Very simple tests to see if the orbital products are computed correctly"""
geometry = geometry.copy()
Expand Down Expand Up @@ -209,7 +208,6 @@ def test_orbital_products(geometry):
assert np.allclose(dens.grid.ravel(), predicted)


@pytest.mark.skip(reason="bug in array indices, data-sc-off (sometimes)")
def test_orbital_products_periodic(geometry):
"""Very simple tests to see if the orbital products are computed correctly
when there are periodic conditions.
Expand Down Expand Up @@ -250,7 +248,6 @@ def test_orbital_products_periodic(geometry):
# We don't check for now.


@pytest.mark.skip(reason="bug in array indices, data-sc-off (sometimes)")
@pytest.mark.parametrize("ncoeffs", [1, 2])
def test_orbital_products_onthefly_reduction(geometry, psi_values, ncoeffs):
"""Checks that the on the fly reduction produces the same
Expand Down

0 comments on commit a12800f

Please sign in to comment.