summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--polymatrix/expression/mixins/additionexprmixin.py2
-rw-r--r--polymatrix/expression/mixins/blockdiagexprmixin.py5
-rw-r--r--polymatrix/expression/mixins/cacheexprmixin.py4
-rw-r--r--polymatrix/expression/mixins/combinationsexprmixin.py2
-rw-r--r--polymatrix/expression/mixins/degreeexprmixin.py3
-rw-r--r--polymatrix/expression/mixins/determinantexprmixin.py14
-rw-r--r--polymatrix/expression/mixins/diagexprmixin.py5
-rw-r--r--polymatrix/expression/mixins/divergenceexprmixin.py4
-rw-r--r--polymatrix/expression/utils/broadcastpolymatrix.py9
-rw-r--r--polymatrix/expression/utils/getvariableindices.py4
-rw-r--r--polymatrix/utils/deprecation.py6
11 files changed, 46 insertions, 12 deletions
diff --git a/polymatrix/expression/mixins/additionexprmixin.py b/polymatrix/expression/mixins/additionexprmixin.py
index bc83ff3..200c328 100644
--- a/polymatrix/expression/mixins/additionexprmixin.py
+++ b/polymatrix/expression/mixins/additionexprmixin.py
@@ -44,6 +44,8 @@ class AdditionExprMixin(ExpressionBaseMixin):
poly_matrix_data = {}
+ # NP: this code is very old I presume, so it iterates over zero entries
+ # FIXME: iterate only over non-zero entries
for row in range(left.shape[0]):
for col in range(left.shape[1]):
poly_data = {}
diff --git a/polymatrix/expression/mixins/blockdiagexprmixin.py b/polymatrix/expression/mixins/blockdiagexprmixin.py
index 4688139..18af1f3 100644
--- a/polymatrix/expression/mixins/blockdiagexprmixin.py
+++ b/polymatrix/expression/mixins/blockdiagexprmixin.py
@@ -23,18 +23,20 @@ class BlockDiagExprMixin(ExpressionBaseMixin):
def apply(
self,
state: ExpressionState,
- ) -> tuple[ExpressionState, PolyMatrix]:
+ ) -> tuple[ExpressionState, PolyMatrix]: # FIXME: incorrect return type
all_underlying = []
for expr in self.underlying:
state, polymat = expr.apply(state=state)
all_underlying.append(polymat)
+ # NP: this is a very weird place to put a class
@dataclassabc.dataclassabc(frozen=True)
class BlockDiagPolyMatrix(PolyMatrixMixin):
all_underlying: tuple[PolyMatrixMixin]
underlying_row_col_range: tuple[tuple[int, int], ...]
shape: tuple[int, int]
+ # FIXME: typing problems
def get_poly(self, row: int, col: int) -> dict[tuple[int, ...], float]:
for polymatrix, ((row_start, col_start), (row_end, col_end)) in zip(
self.all_underlying, self.underlying_row_col_range
@@ -49,6 +51,7 @@ class BlockDiagExprMixin(ExpressionBaseMixin):
else:
return None
+ # NP: Do not raise generic expression, specialize error
raise Exception(f"row {row} is out of bounds")
underlying_row_col_range = tuple(
diff --git a/polymatrix/expression/mixins/cacheexprmixin.py b/polymatrix/expression/mixins/cacheexprmixin.py
index 94cd7dc..7947a74 100644
--- a/polymatrix/expression/mixins/cacheexprmixin.py
+++ b/polymatrix/expression/mixins/cacheexprmixin.py
@@ -12,14 +12,14 @@ class CacheExprMixin(ExpressionBaseMixin):
"""Caches the polynomial matrix using the state"""
@property
- @abc.abstractclassmethod
+ @abc.abstractclassmethod # FIXME: not a classmethod
def underlying(self) -> ExpressionBaseMixin: ...
# overwrites the abstract method of `ExpressionBaseMixin`
def apply(
self,
state: ExpressionStateMixin,
- ) -> tuple[ExpressionStateMixin, PolyMatrixMixin]:
+ ) -> tuple[ExpressionStateMixin, PolyMatrixMixin]: # FIXME: return type
if self in state.cache:
return state, state.cache[self]
diff --git a/polymatrix/expression/mixins/combinationsexprmixin.py b/polymatrix/expression/mixins/combinationsexprmixin.py
index 8a99455..56a1d64 100644
--- a/polymatrix/expression/mixins/combinationsexprmixin.py
+++ b/polymatrix/expression/mixins/combinationsexprmixin.py
@@ -14,6 +14,8 @@ class CombinationsExprMixin(ExpressionBaseMixin):
[[x]] -> [[1], [x], [x**2], [x**3]]
"""
+ # NP: example is not great, should be with x_1 and x_2 to show actual
+ # NP: effect of combinations (or am I understanding this wrong?)
@property
@abc.abstractmethod
diff --git a/polymatrix/expression/mixins/degreeexprmixin.py b/polymatrix/expression/mixins/degreeexprmixin.py
index e847102..5518d0a 100644
--- a/polymatrix/expression/mixins/degreeexprmixin.py
+++ b/polymatrix/expression/mixins/degreeexprmixin.py
@@ -8,6 +8,9 @@ from polymatrix.expressionstate.abc import ExpressionState
from polymatrix.utils.getstacklines import FrameSummary
+# NP: IIRC this gets the highest degree.
+# NP: If I am correct consider renaming to MaxDegree
+# NP: (also what is the use case?)
class DegreeExprMixin(ExpressionBaseMixin):
@property
@abc.abstractmethod
diff --git a/polymatrix/expression/mixins/determinantexprmixin.py b/polymatrix/expression/mixins/determinantexprmixin.py
index 5694ddd..2c034d1 100644
--- a/polymatrix/expression/mixins/determinantexprmixin.py
+++ b/polymatrix/expression/mixins/determinantexprmixin.py
@@ -14,6 +14,7 @@ class DeterminantExprMixin(ExpressionBaseMixin):
@abc.abstractmethod
def underlying(self) -> ExpressionBaseMixin: ...
+ # NP: delete this code?
# # overwrites the abstract method of `ExpressionBaseMixin`
# @property
# def shape(self) -> tuple[int, int]:
@@ -39,13 +40,20 @@ class DeterminantExprMixin(ExpressionBaseMixin):
index_start = state.n_param
rel_index = 0
+ # NP: consider providing a reference eg algorithm name if it has one
+ # NP: or at least a minimal overview / explaination.
+
for row in range(underlying.shape[0]):
+ # FIXME: type annotations of dictionaries inside method
polynomial = collections.defaultdict(float)
# f in f-v^T@x-r^2
# terms = underlying.get_poly(row, row)
try:
underlying_poly = underlying.get_poly(row, row)
+
+ # NP: this is polymatrix leaking the abstraction, you should not
+ # NP: need to care about these problems here. => Fix inside PolyMatrix
except KeyError:
pass
else:
@@ -99,6 +107,12 @@ class DeterminantExprMixin(ExpressionBaseMixin):
rel_index += row
inequality_data[row, 0] = dict(polynomial)
+ # NP: Type checker does not like the fact that state is of type
+ # NP: ExpressionState, while register returns ExpressionStateMixinis this
+ # NP: was a common problem I had while coding mdpoly, that eventually led
+ # NP: me to abandoning mixins because they are a pain to get right with typing
+ # NP: such that the type checker (mypy) is happy. This problem happens all over the
+ # NP: place, I don't have a quick solution.
state = state.register(rel_index)
poly_matrix = init_poly_matrix(
diff --git a/polymatrix/expression/mixins/diagexprmixin.py b/polymatrix/expression/mixins/diagexprmixin.py
index 826cb94..1feec9d 100644
--- a/polymatrix/expression/mixins/diagexprmixin.py
+++ b/polymatrix/expression/mixins/diagexprmixin.py
@@ -20,6 +20,7 @@ class DiagExprMixin(ExpressionBaseMixin):
def underlying(self) -> ExpressionBaseMixin: ...
# overwrites the abstract method of `ExpressionBaseMixin`
+ # FIXME: typing
def apply(
self,
state: ExpressionStateMixin,
@@ -37,6 +38,8 @@ class DiagExprMixin(ExpressionBaseMixin):
if row == col:
return self.underlying.get_poly(row, 0)
else:
+ # FIXME: should return none according to base class
+ # NP: Though returning zero makes more sense
return {tuple(): 0.0}
return state, DiagPolyMatrix(
@@ -45,8 +48,10 @@ class DiagExprMixin(ExpressionBaseMixin):
)
else:
+ # NP: replace assertions with meaningful exception
assert underlying.shape[0] == underlying.shape[1], f"{underlying.shape=}"
+ # NP: why is this called Trace?
@dataclassabc.dataclassabc(frozen=True)
class TracePolyMatrix(PolyMatrixMixin):
underlying: PolyMatrixMixin
diff --git a/polymatrix/expression/mixins/divergenceexprmixin.py b/polymatrix/expression/mixins/divergenceexprmixin.py
index b4deb25..3477ee1 100644
--- a/polymatrix/expression/mixins/divergenceexprmixin.py
+++ b/polymatrix/expression/mixins/divergenceexprmixin.py
@@ -29,6 +29,7 @@ class DivergenceExprMixin(ExpressionBaseMixin):
state, underlying = self.underlying.apply(state=state)
state, variables = get_variable_indices_from_variable(state, self.variables)
+ # NP: replace with exception with error explaination
assert underlying.shape[1] == 1, f"{underlying.shape=}"
assert (
len(variables) == underlying.shape[0]
@@ -37,8 +38,9 @@ class DivergenceExprMixin(ExpressionBaseMixin):
polynomial_data = collections.defaultdict(float)
for row, variable in enumerate(variables):
+ # NP: did you know that python has the walrus operator? `:=`
+ # NP: if poly := underlying.get_poly(row, 0):
polynomial = underlying.get_poly(row, 0)
-
if polynomial is None:
continue
diff --git a/polymatrix/expression/utils/broadcastpolymatrix.py b/polymatrix/expression/utils/broadcastpolymatrix.py
index c24e31e..7bfde13 100644
--- a/polymatrix/expression/utils/broadcastpolymatrix.py
+++ b/polymatrix/expression/utils/broadcastpolymatrix.py
@@ -4,11 +4,10 @@ from polymatrix.utils.tooperatorexception import to_operator_exception
from polymatrix.polymatrix.abc import PolyMatrix
-# NP: "broadcast" does not mean scalar to me cf. BroadcastPolyMatrix
-# NP: broadcast means only to change into something else cf. numpy.broadcast_to
-# NP: consider renaming to broadcast_to_scalar or just get rid of broadcast
-# NP: altogether and call it to_scalar
-# FIXME: typing problems
+# NP: "broadcast" is not well defined cf. BroadcastPolyMatrix
+# NP: Is this supposed to behave exactly like like in numpy?
+# NP: cf. https://numpy.org/doc/stable/user/basics.broadcasting.html
+# FIXME: typing problems, incorrect return type
def broadcast_poly_matrix(
left: PolyMatrix,
right: PolyMatrix,
diff --git a/polymatrix/expression/utils/getvariableindices.py b/polymatrix/expression/utils/getvariableindices.py
index 743f6b9..aebdeb0 100644
--- a/polymatrix/expression/utils/getvariableindices.py
+++ b/polymatrix/expression/utils/getvariableindices.py
@@ -5,7 +5,9 @@ from polymatrix.expressionstate.abc import ExpressionState
from polymatrix.expression.mixins.expressionbasemixin import ExpressionBaseMixin
-# FIXME: typing
+# NP: very confusing name without explaination / context
+# NP: why is this not a feature of expressionstate?
+# FIXME: typing, especially return type
def get_variable_indices_from_variable(
state: ExpressionState,
variable: ExpressionBaseMixin | int | typing.Any,
diff --git a/polymatrix/utils/deprecation.py b/polymatrix/utils/deprecation.py
index c9dea71..3e4302a 100644
--- a/polymatrix/utils/deprecation.py
+++ b/polymatrix/utils/deprecation.py
@@ -1,5 +1,7 @@
-""" Provides tools to mark parts of code a deprecated, for a smoother
-transition to new code. """
+"""
+Provides tools to mark parts of code a deprecated, for a smoother transition to
+new code.
+"""
from __future__ import annotations
from typing import Callable, Any, overload
from warnings import warn
generated by cgit v1.2.1 (git 2.18.0) at 2024-11-22 13:59:38 +0100