(Advanced) Indexing, Slicing, Masking, Reshaping and Transposition for Random Variables and Distributions

[marvinpfoertner]

This PR enables full numpy-style indexing for random variables and distributions (currently Normal and Dirac) as in

https://numpy.org/doc/stable/reference/arrays.indexing.html

Closes #11.

[marvinpfoertner]

I noticed that the .reshape() call on a random variable changes the shape of the random variable in-place.

probnum/src/probnum/prob/randomvariable.py

Lines 193 to 208 in 1ddf55a

	def reshape(self, newshape):
	"""
	Give a new shape to a random variable.
	Parameters
	----------
	newshape : int or tuple of ints
	New shape for the random variable. It must be compatible with the original shape.
	Returns
	-------
	reshaped_rv : ``self`` with the new dimensions of ``shape``.
	"""
	self._shape = newshape
	self._distribution.reshape(newshape=newshape)
	return self

To be consistent with the numpy API, I would propose we change this to return a new random variable with the desired shape.

Any thoughts @JonathanWenger, @pnkraemer, @nathanaelbosch?

[JonathanWenger]

I noticed that the .reshape() call on a random variable changes the shape of the random variable in-place.

probnum/src/probnum/prob/randomvariable.py

Lines 193 to 208 in 1ddf55a

	def reshape(self, newshape):
	"""
	Give a new shape to a random variable.
	Parameters
	----------
	newshape : int or tuple of ints
	New shape for the random variable. It must be compatible with the original shape.
	Returns
	-------
	reshaped_rv : ``self`` with the new dimensions of ``shape``.
	"""
	self._shape = newshape
	self._distribution.reshape(newshape=newshape)
	return self

To be consistent with the numpy API, I would propose we change this to return a new random variable with the desired shape.

Any thoughts @JonathanWenger, @pnkraemer, @nathanaelbosch?

In general we should be consistent with the numpy API, so I agree that .reshape() should not perform the operation in place, but return a new random variable. However rv.shape = (a,b) should perform the operation in place to be consistent with NumPy.

It seems to be a bit more complicated (see also: https://stackoverflow.com/questions/46574568/does-numpy-reshape-create-a-copy). While some_array.reshape(size) creates a new object (a so-called view) it still shares the same data buffer, which is not the same as reshape creating a new array.

[marvinpfoertner]

I noticed that the .reshape() call on a random variable changes the shape of the random variable in-place.

probnum/src/probnum/prob/randomvariable.py

Lines 193 to 208 in 1ddf55a

	def reshape(self, newshape):
	"""
	Give a new shape to a random variable.
	Parameters
	----------
	newshape : int or tuple of ints
	New shape for the random variable. It must be compatible with the original shape.
	Returns
	-------
	reshaped_rv : ``self`` with the new dimensions of ``shape``.
	"""
	self._shape = newshape
	self._distribution.reshape(newshape=newshape)
	return self

To be consistent with the numpy API, I would propose we change this to return a new random variable with the desired shape.
Any thoughts @JonathanWenger, @pnkraemer, @nathanaelbosch?

In general we should be consistent with the numpy API, so I agree that .reshape() should not perform the operation in place, but return a new random variable. However rv.shape = (a,b) should perform the operation in place to be consistent with NumPy.

It seems to be a bit more complicated (see also: https://stackoverflow.com/questions/46574568/does-numpy-reshape-create-a-copy). While some_array.reshape(size) creates a new object (a so-called view) it still shares the same data buffer, which is not the same as reshape creating a new array.

I agree but since we'll call numpy's resize internally, our random variables inherit the same data buffer. Actually I'm pretty sure that a view is essentially a new array which just shares the pointer to the contiguous piece of memory of the old array.

[JonathanWenger]

I noticed that the .reshape() call on a random variable changes the shape of the random variable in-place.

probnum/src/probnum/prob/randomvariable.py

Lines 193 to 208 in 1ddf55a

	def reshape(self, newshape):
	"""
	Give a new shape to a random variable.
	Parameters
	----------
	newshape : int or tuple of ints
	New shape for the random variable. It must be compatible with the original shape.
	Returns
	-------
	reshaped_rv : ``self`` with the new dimensions of ``shape``.
	"""
	self._shape = newshape
	self._distribution.reshape(newshape=newshape)
	return self

To be consistent with the numpy API, I would propose we change this to return a new random variable with the desired shape.
Any thoughts @JonathanWenger, @pnkraemer, @nathanaelbosch?

In general we should be consistent with the numpy API, so I agree that .reshape() should not perform the operation in place, but return a new random variable. However rv.shape = (a,b) should perform the operation in place to be consistent with NumPy.
It seems to be a bit more complicated (see also: https://stackoverflow.com/questions/46574568/does-numpy-reshape-create-a-copy). While some_array.reshape(size) creates a new object (a so-called view) it still shares the same data buffer, which is not the same as reshape creating a new array.

I agree but since we'll call numpy's resize internally, our random variables inherit the same data buffer. Actually I'm pretty sure that a view is essentially a new array which just shares the pointer to the contiguous piece of memory of the old array.

That's also my understanding from a quick scan of the topic.

[marvinpfoertner]

I noticed that the .reshape() call on a random variable changes the shape of the random variable in-place.

probnum/src/probnum/prob/randomvariable.py

Lines 193 to 208 in 1ddf55a

	def reshape(self, newshape):
	"""
	Give a new shape to a random variable.
	Parameters
	----------
	newshape : int or tuple of ints
	New shape for the random variable. It must be compatible with the original shape.
	Returns
	-------
	reshaped_rv : ``self`` with the new dimensions of ``shape``.
	"""
	self._shape = newshape
	self._distribution.reshape(newshape=newshape)
	return self

To be consistent with the numpy API, I would propose we change this to return a new random variable with the desired shape.
Any thoughts @JonathanWenger, @pnkraemer, @nathanaelbosch?

In general we should be consistent with the numpy API, so I agree that .reshape() should not perform the operation in place, but return a new random variable. However rv.shape = (a,b) should perform the operation in place to be consistent with NumPy.
It seems to be a bit more complicated (see also: https://stackoverflow.com/questions/46574568/does-numpy-reshape-create-a-copy). While some_array.reshape(size) creates a new object (a so-called view) it still shares the same data buffer, which is not the same as reshape creating a new array.

I agree but since we'll call numpy's resize internally, our random variables inherit the same data buffer. Actually I'm pretty sure that a view is essentially a new array which just shares the pointer to the contiguous piece of memory of the old array.

That's also my understanding from a quick scan of the topic.

Should I make these changes in this PR?

[JonathanWenger]

Fine with me.

[marvinpfoertner]

Fine with me.

Done

[JonathanWenger]

Great job on the PR! I saw some things that will surely changed based on the discussion surrounding the removal of Distribution. So just keep those in mind before you invest too much time into making improvements based on my comments.

[marvinpfoertner]

Inplace reshaping for univariate normals is actually impossible due to the fact that the attributes of numpy's array scalars are immutable. See also https://numpy.org/doc/stable/reference/arrays.scalars.html. We should overthink #130. It would be best if scalar quantities behave exactly the same as arrays. Maybe we should use arrays of shape () internally?

[marvinpfoertner]

@JonathanWenger

[JonathanWenger]

Yes i think representing everything internally as arrays makes a lot of sense. This removes quite a few annoying conditionals in the code as well.

[marvinpfoertner]

Alright, I'll rephrase #130 then?

[marvinpfoertner]

@JonathanWenger I resolved all your comments. The PR would be ready to merge. Any objections?