validate diag coulomb matrices only upper triangular

python/ffsim/linalg/double_factorized_decomposition.py

@@ -78,6 +78,17 @@ def modified_cholesky(
     return cholesky_vecs[:, :index]
 
 
+def _validate_diag_coulomb_indices(indices: list[tuple[int, int]] | None):
+    if indices is not None:
+        for i, j in indices:
+            if i > j:
+                raise ValueError(
+                    "When specifying diagonal Coulomb indices, you must give only "
+                    "upper trianglular indices. "
+                    f"Got {(i, j)}, which is a lower triangular index."
+                )
+
+
 def double_factorized(
     two_body_tensor: np.ndarray,
     *,
@@ -173,13 +184,7 @@ def double_factorized(
     .. _arXiv:2104.08957: https://arxiv.org/abs/2104.08957
     .. _scipy.optimize.minimize: https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize.html
     """
-    if diag_coulomb_indices is not None:
-        for i, j in diag_coulomb_indices:
-            if i > j:
-                raise ValueError(
-                    "diag_coulomb_indices must contain only upper triangular indices. "
-                    f"Got {(i, j)}, which is a lower triangle index."
-                )
+    _validate_diag_coulomb_indices(diag_coulomb_indices)
 
     n_modes, _, _, _ = two_body_tensor.shape
     if max_vecs is None:

python/ffsim/variational/ucj.py

@@ -28,6 +28,17 @@
 )
 
 
+def _validate_diag_coulomb_indices(indices: list[tuple[int, int]] | None):
+    if indices is not None:
+        for i, j in indices:
+            if i > j:
+                raise ValueError(
+                    "When specifying diagonal Coulomb indices, you must give only "
+                    "upper trianglular indices. "
+                    f"Got {(i, j)}, which is a lower triangular index."
+                )
+
+
 @dataclass(frozen=True)
 class UCJOperator:
     r"""A unitary cluster Jastrow operator.
@@ -89,6 +100,8 @@ def n_params(
         with_final_orbital_rotation: bool = False,
     ) -> int:
         """Return the number of parameters of an ansatz with given settings."""
+        _validate_diag_coulomb_indices(alpha_alpha_indices)
+        _validate_diag_coulomb_indices(alpha_beta_indices)
         n_params_aa = (
             norb * (norb + 1) // 2
             if alpha_alpha_indices is None
@@ -124,15 +137,27 @@ def from_parameters(
             alpha_alpha_indices: Allowed indices for nonzero values of the "alpha-alpha"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             alpha_beta_indices: Allowed indices for nonzero values of the "alpha-beta"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             with_final_orbital_rotation: Whether to include a final orbital rotation
                 in the operator.
 
         Returns:
             The UCJ operator constructed from the given parameters.
+
+        Raises:
+            ValueError: alpha_alpha_indices contains lower triangular indices.
+            ValueError: alpha_beta_indices contains lower triangular indices.
         """
+        _validate_diag_coulomb_indices(alpha_alpha_indices)
+        _validate_diag_coulomb_indices(alpha_beta_indices)
         return UCJOperator(
             *_ucj_from_parameters(
                 params,
@@ -161,13 +186,25 @@ def to_parameters(
             alpha_alpha_indices: Allowed indices for nonzero values of the "alpha-alpha"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             alpha_beta_indices: Allowed indices for nonzero values of the "alpha-beta"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
 
         Returns:
             The real-valued parameter vector.
+
+        Raises:
+            ValueError: alpha_alpha_indices contains lower triangular indices.
+            ValueError: alpha_beta_indices contains lower triangular indices.
         """
+        _validate_diag_coulomb_indices(alpha_alpha_indices)
+        _validate_diag_coulomb_indices(alpha_beta_indices)
         return _ucj_to_parameters(
             diag_coulomb_mats_alpha_alpha=self.diag_coulomb_mats_alpha_alpha,
             diag_coulomb_mats_alpha_beta=self.diag_coulomb_mats_alpha_beta,
@@ -367,15 +404,27 @@ def from_parameters(
             alpha_alpha_indices: Allowed indices for nonzero values of the "alpha-alpha"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             alpha_beta_indices: Allowed indices for nonzero values of the "alpha-beta"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             with_final_orbital_rotation: Whether to include a final orbital rotation
                 in the operator.
 
         Returns:
             The real UCJ operator constructed from the given parameters.
+
+        Raises:
+            ValueError: alpha_alpha_indices contains lower triangular indices.
+            ValueError: alpha_beta_indices contains lower triangular indices.
         """
+        _validate_diag_coulomb_indices(alpha_alpha_indices)
+        _validate_diag_coulomb_indices(alpha_beta_indices)
         return RealUCJOperator(
             *_ucj_from_parameters(
                 params,
@@ -404,13 +453,25 @@ def to_parameters(
             alpha_alpha_indices: Allowed indices for nonzero values of the "alpha-alpha"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
             alpha_beta_indices: Allowed indices for nonzero values of the "alpha-beta"
                 diagonal Coulomb matrices (see the docstring of this class).
                 If not specified, all matrix entries are allowed to be nonzero.
+                This list should contain only upper trianglular indices, i.e.,
+                pairs :math:`(i, j)` where :math:`i \leq j`. Passing a list with
+                lower triangular indices will raise an error.
 
         Returns:
             The real-valued parameter vector.
+
+        Raises:
+            ValueError: alpha_alpha_indices contains lower triangular indices.
+            ValueError: alpha_beta_indices contains lower triangular indices.
         """
+        _validate_diag_coulomb_indices(alpha_alpha_indices)
+        _validate_diag_coulomb_indices(alpha_beta_indices)
         return _ucj_to_parameters(
             diag_coulomb_mats_alpha_alpha=self.diag_coulomb_mats_alpha_alpha,
             diag_coulomb_mats_alpha_beta=self.diag_coulomb_mats_alpha_beta,

tests/variational/ucj_test.py

@@ -14,6 +14,7 @@
 
 import numpy as np
 import pyscf
+import pytest
 import scipy.linalg
 from pyscf import cc
 
@@ -73,6 +74,21 @@ def test_n_params():
         )
         assert actual == expected
 
+        with pytest.raises(ValueError, match="triangular"):
+            actual = ffsim.UCJOperator.n_params(
+                norb,
+                n_reps,
+                alpha_alpha_indices=[(1, 0)],
+                alpha_beta_indices=alpha_beta_indices,
+            )
+        with pytest.raises(ValueError, match="triangular"):
+            actual = ffsim.UCJOperator.n_params(
+                norb,
+                n_reps,
+                alpha_alpha_indices=alpha_alpha_indices,
+                alpha_beta_indices=[(1, 0)],
+            )
+
 
 def test_parameters_roundtrip():
     norb = 5