Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indexing of multi-dimensional spaces #965

Open
kohr-h opened this issue Mar 17, 2017 · 4 comments
Open

Indexing of multi-dimensional spaces #965

kohr-h opened this issue Mar 17, 2017 · 4 comments

Comments

@kohr-h
Copy link
Member

kohr-h commented Mar 17, 2017

When indexing spaces with ndim axes, it would be natural to read the axes in left-to-right order. What I mean by that is this:

>>> space.shape == shape
True
>>> space[0].shape == shape[1:]
True
>>> space[0, 0].shape == space[0][0].shape == shape[2:]
True

and so on. Now the top one and the right part of the bottom one are as in ProductSpace, except that it doesn't implement shape, but conceptually that's how it is.

Question: Should we extend ProductSpace.__getitem__ to support tuple indexing by just doing this:

def __getitem__(self, indices):
    ...
    if isinstance(indices, tuple):
        if not indices:
            return self
        idx = indices[0]
        if isinstance(idx, Number):
            return self.spaces[idx][indices[1:]]
        elif isinstance(idx, slice):
            spaces = self.spaces[idx]
            return ProductSpace(*(space[indices[1:]]
                                  for space in spaces))
        else:
            raise TypeError('index tuple can only contain'
                            'ints or slices')
@kohr-h
Copy link
Member Author

kohr-h commented Mar 17, 2017

Of course, the question is where to stop. Probably it would be sensible to raise an error when trying to use a remaining index for a non-productspace.

@adler-j
Copy link
Member

adler-j commented Mar 18, 2017

Your example makes perfect sense to me, and I agree that we want that.

With that said, we really want #157 now that we have true tensor spaces, and this could be part of that.

@kohr-h
Copy link
Member Author

kohr-h commented Mar 18, 2017

Agreed. This is now done, and I'll mirror this behavior for the indexing of TensorSpace.

@kohr-h
Copy link
Member Author

kohr-h commented Aug 10, 2017

I've commented in #861 about this, here's an update:

When indexing spaces there's always the ambiguity of indexing the "imaginary" array that the resulting space should wrap vs. indexing by axis.

The former is of limited use in TensorSpace type spaces apart from a somewhat convenient flow for indexing of tensors (index space and array the same way, then wrap the resulting array with the indexed space).
In contrast, for DiscreteLp that indexing is interesting since the partition carries information on each space point that indexing will transport.
Another interesting case is FunctionSpace with tensor-valued functions. Indexing should logically affect the output dimensions, but any indexing except by axis doesn't really make sense.

So anyway, I think that due to a not entirely clear-cut situation, the default should be to put indexing by axis on a byaxis property and decide later what to do in __getitem__.

For FunctionSpace, and by consequence also later for DiscreteLp, indexing can happen by input or output dimensions, so there will be byaxis_in and byaxis_out.

kohr-h pushed a commit to kohr-h/odl that referenced this issue Sep 21, 2017
kohr-h pushed a commit to kohr-h/odl that referenced this issue Oct 5, 2017
kohr-h pushed a commit to kohr-h/odl that referenced this issue Dec 11, 2017
Changes in detail:
- Add dtype with shape to DiscreteLp (mostly __repr__, factory
  functions and some downstream methods). As a consequence,
  `shape_[in,out]` and `ndim_[in,out]` are added for the
  different types of axes, as well as `scalar_dtype`.
- Add `PerAxisWeighting` and make it the default for
  `DiscreteLp` type spaces. Reason: this way the `tspace`
  knows how to deal with removed axes etc. This is important
  for a smooth experience with indexing and reductions over
  axes.
  Helpers for slicing weightings help structure this task.
- Implement `__getitem__` for `TensorSpace` and `DiscreteLp`,
  including (hopefully) reasonable propagation of weights.
  The new `simulate_slicing` utility function simplifies
  this task.
- Allow indexing with ODL tensors of boolean or integer dtype.
- Implement correct weighting for backprojections with
  non-uniform angles, using per-axis weighting and a new
  helper `adjoint_weightings` to apply the weightings in an
  efficient way.
  The correct weighting from the range of `RayTransform`
  is determined by the new `proj_space_weighting` helper.
- Change the space `_*_impl` methods to always expect and
  return Numpy arrays, and adapt the calling code.
- Change behavior of `norm` and `dist` to ignoring weights
  for `exponent=inf`, in accordance with math.
- Improve speed of `all_equal` for comparison of arrays.
- Account for `None` entries in indices in the
  `normalized_index_expression` helper, thus allowing
  creation of new axes.
- Remove `dicsr_sequence_space`, it was largely unused and
  just a maintenance burden. Use a regular `uniform-discr`
  from zero to `shape` instead.
- Remove `Weighting.equiv()` mehtods, never used and hard
  to maintain (n^2 possibilities).
- Remove the (largely useless) `_weighting` helper to create
  weighting instances since it would have been ambiguous
  with sequences of scalars (array or per axis?). Also remove
  the `npy_weighted_*` functions, they were useless, too.
- Remove some dead code from tomo/util.
- A bunch of minor fixes, as usual.

Closes: odlgroup#908, odlgroup#907, odlgroup#1113, odlgroup#965, odlgroup#286, odlgroup#267, odlgroup#1001
kohr-h pushed a commit to kohr-h/odl that referenced this issue Dec 11, 2017
Changes in detail:
- Add dtype with shape to DiscreteLp (mostly __repr__, factory
  functions and some downstream methods). As a consequence,
  `shape_[in,out]` and `ndim_[in,out]` are added for the
  different types of axes, as well as `scalar_dtype`.
- Add `PerAxisWeighting` and make it the default for
  `DiscreteLp` type spaces. Reason: this way the `tspace`
  knows how to deal with removed axes etc. This is important
  for a smooth experience with indexing and reductions over
  axes.
  Helpers for slicing weightings help structure this task.
- Implement `__getitem__` for `TensorSpace` and `DiscreteLp`,
  including (hopefully) reasonable propagation of weights.
  The new `simulate_slicing` utility function simplifies
  this task.
- Allow indexing with ODL tensors of boolean or integer dtype.
- Implement correct weighting for backprojections with
  non-uniform angles, using per-axis weighting and a new
  helper `adjoint_weightings` to apply the weightings in an
  efficient way.
  The correct weighting from the range of `RayTransform`
  is determined by the new `proj_space_weighting` helper.
- Change the space `_*_impl` methods to always expect and
  return Numpy arrays, and adapt the calling code.
- Change behavior of `norm` and `dist` to ignoring weights
  for `exponent=inf`, in accordance with math.
- Improve speed of `all_equal` for comparison of arrays.
- Account for `None` entries in indices in the
  `normalized_index_expression` helper, thus allowing
  creation of new axes.
- Remove `dicsr_sequence_space`, it was largely unused and
  just a maintenance burden. Use a regular `uniform-discr`
  from zero to `shape` instead.
- Remove `Weighting.equiv()` mehtods, never used and hard
  to maintain (n^2 possibilities).
- Remove the (largely useless) `_weighting` helper to create
  weighting instances since it would have been ambiguous
  with sequences of scalars (array or per axis?). Also remove
  the `npy_weighted_*` functions, they were useless, too.
- Remove some dead code from tomo/util.
- A bunch of minor fixes, as usual.

Closes: odlgroup#908
Closes: odlgroup#907
Closes: odlgroup#1113
Closes: odlgroup#965
Closes: odlgroup#286
Closes: odlgroup#267
Closes: odlgroup#1001
kohr-h pushed a commit to kohr-h/odl that referenced this issue Jun 30, 2018
Changes in detail:
- Add dtype with shape to DiscreteLp (mostly __repr__, factory
  functions and some downstream methods). As a consequence,
  `shape_[in,out]` and `ndim_[in,out]` are added for the
  different types of axes, as well as `scalar_dtype`.
- Add `PerAxisWeighting` and make it the default for
  `DiscreteLp` type spaces. Reason: this way the `tspace`
  knows how to deal with removed axes etc. This is important
  for a smooth experience with indexing and reductions over
  axes.
  Helpers for slicing weightings help structure this task.
- Implement `__getitem__` for `TensorSpace` and `DiscreteLp`,
  including (hopefully) reasonable propagation of weights.
  The new `simulate_slicing` utility function simplifies
  this task.
- Allow indexing with ODL tensors of boolean or integer dtype.
- Implement correct weighting for backprojections with
  non-uniform angles, using per-axis weighting and a new
  helper `adjoint_weightings` to apply the weightings in an
  efficient way.
  The correct weighting from the range of `RayTransform`
  is determined by the new `proj_space_weighting` helper.
- Change the space `_*_impl` methods to always expect and
  return Numpy arrays, and adapt the calling code.
- Change behavior of `norm` and `dist` to ignoring weights
  for `exponent=inf`, in accordance with math.
- Improve speed of `all_equal` for comparison of arrays.
- Account for `None` entries in indices in the
  `normalized_index_expression` helper, thus allowing
  creation of new axes.
- Remove `dicsr_sequence_space`, it was largely unused and
  just a maintenance burden. Use a regular `uniform-discr`
  from zero to `shape` instead.
- Remove `Weighting.equiv()` mehtods, never used and hard
  to maintain (n^2 possibilities).
- Remove the (largely useless) `_weighting` helper to create
  weighting instances since it would have been ambiguous
  with sequences of scalars (array or per axis?). Also remove
  the `npy_weighted_*` functions, they were useless, too.
- Remove some dead code from tomo/util.
- A bunch of minor fixes, as usual.

Closes: odlgroup#908
Closes: odlgroup#907
Closes: odlgroup#1113
Closes: odlgroup#965
Closes: odlgroup#286
Closes: odlgroup#267
Closes: odlgroup#1001
kohr-h pushed a commit to kohr-h/odl that referenced this issue Sep 12, 2018
Changes in detail:
- Add dtype with shape to DiscreteLp (mostly __repr__, factory
  functions and some downstream methods). As a consequence,
  `shape_[in,out]` and `ndim_[in,out]` are added for the
  different types of axes, as well as `scalar_dtype`.
- Add `PerAxisWeighting` and make it the default for
  `DiscreteLp` type spaces. Reason: this way the `tspace`
  knows how to deal with removed axes etc. This is important
  for a smooth experience with indexing and reductions over
  axes.
  Helpers for slicing weightings help structure this task.
- Implement `__getitem__` for `TensorSpace` and `DiscreteLp`,
  including (hopefully) reasonable propagation of weights.
  The new `simulate_slicing` utility function simplifies
  this task.
- Allow indexing with ODL tensors of boolean or integer dtype.
- Implement correct weighting for backprojections with
  non-uniform angles, using per-axis weighting and a new
  helper `adjoint_weightings` to apply the weightings in an
  efficient way.
  The correct weighting from the range of `RayTransform`
  is determined by the new `proj_space_weighting` helper.
- Change the space `_*_impl` methods to always expect and
  return Numpy arrays, and adapt the calling code.
- Change behavior of `norm` and `dist` to ignoring weights
  for `exponent=inf`, in accordance with math.
- Improve speed of `all_equal` for comparison of arrays.
- Account for `None` entries in indices in the
  `normalized_index_expression` helper, thus allowing
  creation of new axes.
- Remove `dicsr_sequence_space`, it was largely unused and
  just a maintenance burden. Use a regular `uniform-discr`
  from zero to `shape` instead.
- Remove `Weighting.equiv()` mehtods, never used and hard
  to maintain (n^2 possibilities).
- Remove the (largely useless) `_weighting` helper to create
  weighting instances since it would have been ambiguous
  with sequences of scalars (array or per axis?). Also remove
  the `npy_weighted_*` functions, they were useless, too.
- Remove some dead code from tomo/util.
- A bunch of minor fixes, as usual.

Closes: odlgroup#908
Closes: odlgroup#907
Closes: odlgroup#1113
Closes: odlgroup#965
Closes: odlgroup#286
Closes: odlgroup#267
Closes: odlgroup#1001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants