Skip to content

Commit

Permalink
math.Vectorize accepts a list of sequences during the initialization.…
Browse files Browse the repository at this point in the history
… Fixes a bug (undetected in tests, present in analysis) when instance method failed to be patched runtime.
  • Loading branch information
ynikitenko committed Jul 31, 2023
1 parent 19ed394 commit 765b59e
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 21 deletions.
62 changes: 43 additions & 19 deletions lena/math/elements.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,38 +263,60 @@ def __init__(self, seq, construct=None, dim=-1):
However, often an object constructor can allow
an arbitrary dimension (like ``tuple``).
In that case, provide *dim*.
*seq* can be a list of :class:`.FillComputeSeq`-s.
In that case dimension should not be provided.
"""
default_dim = -1
# todo: if needed, a list *seq* could mean
# a list of sequences of the needed dimension.
try:
self._seq = lena.core.FillComputeSeq(seq)
except TypeError:
raise lena.core.LenaTypeError(
"seq must be convertible to FillComputeSeq"
)

if dim == -1 and construct is not None:
if isinstance(seq, list):
# Vectorize should be a rigid element
# (we don't change its dimension easily),
# but list is associated with parellelism in Lena
# seq must consist of FillComputeSeq-s,
# we don't init them automatically here
self._seqs = seq
assert dim == default_dim
dim = len(seq)
self.fill = self._fill_others
else:
try:
_tmp = construct()
dim = len(_tmp)
self._seq = lena.core.FillComputeSeq(seq)
except TypeError:
# we have a chance to get data dimension from flow
raise lena.core.LenaTypeError(
"seq must be convertible to FillComputeSeq"
)
if dim == default_dim:
pass
# self.fill = self._fill_first
else:
self._seqs = [self._seq]
self._seqs.extend([copy.deepcopy(self._seq) for _ in range(dim-1)])
# self.fill = self._fill_others

## No need to get dim from here. Explicit dim would never hurt.
# if dim == default_dim and construct is not None:
# try:
# _tmp = construct()
# dim = len(_tmp)
# except TypeError:
# # we have a chance to get data dimension from flow
# pass

if dim != -1:
self._seqs = [self._seq]
self._seqs.extend([copy.deepcopy(self._seq) for _ in range(dim-1)])
self.fill = self._fill_others
else:
self.fill = self._fill_first

self._construct = construct
self._dim = dim
self._cur_context = {}
self._filled_once = False

def fill(self, val):
"""Fill sequences for each component of the data vector."""
raise NotImplementedError
# this may be not efficient, but I could not change the method runtime
if self._filled_once:
self._fill_others(val)
else:
self._fill_first(val)

def _fill_first(self, val):
# fill the first element. Will learn data type from that,
Expand All @@ -315,8 +337,10 @@ def _fill_first(self, val):

self._seqs = [self._seq]
self._seqs.extend([copy.deepcopy(self._seq) for _ in range(dim-1)])
# doesn't work. _fill_first is always called (then _fill_others below)
self.fill = self._fill_others
self.fill(val)
self._fill_others(val)
self._filled_once = True

def _fill_others(self, val):
data, context = lena.flow.get_data_context(val)
Expand Down
11 changes: 9 additions & 2 deletions tests/math/test_elements.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,18 @@ def test_vectorize_init():
min_size=1,
)
)
def test_vectorize_hypothesis(data):
@pytest.mark.parametrize("from_seq", [True, False])
def test_vectorize_hypothesis(from_seq, data):
# Vectorize doesn't mess with its input data.
# If we filled values, they will be properly handled
# by the nested sequence.
v = Vectorize(StoreFilled())
if from_seq:
# initializing each sequence explicitly
v = Vectorize([StoreFilled() for _ in range(3)])
else:
# copied automatically when getting dimension from data
v = Vectorize(StoreFilled())

for val in data:
v.fill(val)
result = list(v.compute())
Expand Down

0 comments on commit 765b59e

Please sign in to comment.