Skip to content

Commit

Permalink
feat: improve the SparseLabelOp.register_length handling
Browse files Browse the repository at this point in the history
Closes #1190
  • Loading branch information
mrossinek committed Sep 4, 2023
1 parent 9b48d54 commit 1dd1cdd
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 11 deletions.
6 changes: 5 additions & 1 deletion qiskit_nature/second_q/operators/bosonic_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ def __init__(
super().__init__(data, copy=copy, validate=validate)

@property
def register_length(self) -> int | None:
def register_length(self) -> int:
if self.num_modes is None:
max_index = max(int(term[2:]) for key in self._data for term in key.split())
return max_index + 1

return self.num_modes

def _new_instance(
Expand Down
6 changes: 5 additions & 1 deletion qiskit_nature/second_q/operators/fermionic_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,11 @@ def __init__(
super().__init__(data, copy=copy, validate=validate)

@property
def register_length(self) -> int | None:
def register_length(self) -> int:
if self.num_spin_orbitals is None:
max_index = max(int(term[2:]) for key in self._data for term in key.split())
return max_index + 1

return self.num_spin_orbitals

def _new_instance(
Expand Down
2 changes: 1 addition & 1 deletion qiskit_nature/second_q/operators/sparse_label_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def __init__(

@property
@abstractmethod
def register_length(self) -> int | None:
def register_length(self) -> int:
"""Returns the register length"""

@abstractmethod
Expand Down
6 changes: 5 additions & 1 deletion qiskit_nature/second_q/operators/spin_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ def __init__(
super().__init__(data, copy=copy, validate=validate)

@property
def register_length(self) -> int | None:
def register_length(self) -> int:
if self.num_spins is None:
max_index = max(int(term[2:]) for key in self._data for term in key.split())
return max_index + 1

return self.num_spins

def _new_instance(self, data: Mapping[str, _TCoeff], *, other: SpinOp | None = None) -> SpinOp:
Expand Down
31 changes: 24 additions & 7 deletions qiskit_nature/second_q/operators/vibrational_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def __init__(
data: the operator data, mapping string-based keys to numerical values.
num_modals: number of modals - described by a sequence of integers where each integer
describes the number of modals in the corresponding mode; the total number of modals
defines a ``register_length``.
defines the ``register_length``.
copy: when set to False the `data` will not be copied and the dictionary will be
stored by reference rather than by value (which is the default; ``copy=True``). Note,
that this requires you to not change the contents of the dictionary after
Expand All @@ -194,7 +194,7 @@ def __init__(
super().__init__(data, copy=copy, validate=validate)

@property
def num_modals(self) -> Sequence[int]:
def num_modals(self) -> Sequence[int] | None:
"""The number of modals for each mode on which this operator acts.
This is an optional sequence of integers which are considered lower bounds. That means that
Expand All @@ -207,11 +207,27 @@ def num_modals(self) -> Sequence[int]:

@num_modals.setter
def num_modals(self, num_modals: Sequence[int] | None):
self._num_modals = list(num_modals) if num_modals is not None else []
self._num_modals = list(num_modals) if num_modals is not None else None

@property
def register_length(self) -> int | None:
return sum(self.num_modals) if self.num_modals is not None else None
def register_length(self) -> int:
if self._num_modals is None:
num_modals: list[int] = []
for key in self._data:
for term in key.split():
_, mode_index_str, modal_index_str = term.split("_")
mode_index = int(mode_index_str)
modal_index = int(modal_index_str)

if mode_index + 1 > len(num_modals):
num_modals += [0] * (mode_index + 1 - len(num_modals))

if modal_index > num_modals[mode_index] - 1:
num_modals[mode_index] = modal_index + 1

return sum(num_modals)

return sum(self.num_modals)

def _new_instance(
self, data: Mapping[str, _TCoeff], *, other: VibrationalOp | None = None
Expand All @@ -228,13 +244,14 @@ def pad_to_length(a, b):
def elementwise_max(a, b):
return [max(i, j) for i, j in zip(*pad_to_length(a, b))]

num_modals = elementwise_max(num_modals, other_num_modals)
if num_modals is not None and other_num_modals is not None:
num_modals = elementwise_max(num_modals, other_num_modals)

return self.__class__(data, copy=False, num_modals=num_modals)

def _validate_keys(self, keys: Collection[str]) -> None:
super()._validate_keys(keys)
num_modals = list(self.num_modals)
num_modals = self._num_modals if self._num_modals is not None else []

for key in keys:
# 0. explicitly allow the empty key
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
features:
- |
The :attr:`.SparseLabelOp.register_length` attribute (and by extension that of its subclasses, too)
can no longer take ``None`` as its value. However, the attributes which this one might rely on
(e.g. :attr:`.FermionicOp.num_spin_orbitals` or :attr:`.BosonicOp.num_modes`) can remain ``None``.
This ensures that the lower-bound behavior works as intended.
6 changes: 6 additions & 0 deletions test/second_q/operators/test_bosonic_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,12 @@ def test_permute_indices(self):

self.assertEqual(permuted_op, BosonicOp({"+_2 -_1": 1, "+_1 -_3": 2}, num_modes=4))

def test_reg_len_with_skipped_key_validation(self):
"""Test the behavior of `register_length` after key validation was skipped."""
new_op = BosonicOp({"+_0 -_1": 1}, validate=False)
self.assertIsNone(new_op.num_modes)
self.assertEqual(new_op.register_length, 2)


if __name__ == "__main__":
unittest.main()
6 changes: 6 additions & 0 deletions test/second_q/operators/test_fermionic_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,12 @@ def test_permute_indices(self):
permuted_op, FermionicOp({"+_2 -_1": 1, "+_1 -_3": 2}, num_spin_orbitals=4)
)

def test_reg_len_with_skipped_key_validation(self):
"""Test the behavior of `register_length` after key validation was skipped."""
new_op = FermionicOp({"+_0 -_1": 1}, validate=False)
self.assertIsNone(new_op.num_spin_orbitals)
self.assertEqual(new_op.register_length, 2)


if __name__ == "__main__":
unittest.main()
6 changes: 6 additions & 0 deletions test/second_q/operators/test_spin_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ def test_permute_indices(self):

self.assertEqual(permuted_op, SpinOp({"X_2 Y_1": 1, "Z_1 X_3": 2}, num_spins=4))

def test_reg_len_with_skipped_key_validation(self):
"""Test the behavior of `register_length` after key validation was skipped."""
new_op = SpinOp({"X_0 Y_1": 1}, validate=False)
self.assertIsNone(new_op.num_spins)
self.assertEqual(new_op.register_length, 2)


if __name__ == "__main__":
unittest.main()
6 changes: 6 additions & 0 deletions test/second_q/operators/test_vibrational_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ def test_permute_indices(self):
with self.assertRaises(NotImplementedError):
VibrationalOp({"+_0_0 -_1_0": 2}).permute_indices([1, 0])

def test_reg_len_with_skipped_key_validation(self):
"""Test the behavior of `register_length` after key validation was skipped."""
new_op = VibrationalOp({"+_0_0 -_1_1": 1}, validate=False)
self.assertIsNone(new_op.num_modals)
self.assertEqual(new_op.register_length, 3)


if __name__ == "__main__":
unittest.main()

0 comments on commit 1dd1cdd

Please sign in to comment.