ENH: allow arrays for start and stop in {lin,log,geom}space #12388

mhvk · 2018-11-14T19:26:11Z

As discussed on the mailing list, perhaps it would be good for linspace to allow start and stop to be arrays, which are broadcast against each other. For now, this is just a proof of concept - which presumes the final axis that is counted is the first one. With it, two tests failed, which seem both false positives (in that the tests were not written very well, in one case with a note stating that).

EDIT: TODO from mailing: add axis argument (probably with 0 as a default; should be axis in output, so -1 is last). Could be simple transpose at end, or use reshaping of inputs.

Anyway, with this:

In [4]: np.linspace(np.array([1., 2.]), np.array([3., 4.]), 5)
Out[4]: 
array([[1. , 2. ],
       [1.5, 2.5],
       [2. , 3. ],
       [2.5, 3.5],
       [3. , 4. ]])

In [5]: np.linspace(np.array([1., 2.]), np.array([[3.], [4.]]), 5)
Out[5]: 
array([[[1.  , 2.  ],
        [1.  , 2.  ]],

       [[1.5 , 2.25],
        [1.75, 2.5 ]],

       [[2.  , 2.5 ],
        [2.5 , 3.  ]],

       [[2.5 , 2.75],
        [3.25, 3.5 ]],

       [[3.  , 3.  ],
        [4.  , 4.  ]]])

mhvk · 2018-11-14T19:26:47Z

numpy/core/tests/test_function_base.py

-                # Ideally should be `'shape': ()` but the current interface
-                # does not allow that
-                return {'shape': (1,), 'typestr': '<i4', 'data': self._data,
+                return {'shape': (), 'typestr': '<i4', 'data': self._data,


Apparently, the interface is now better, since shape=() works at least on my python installation.

mhvk · 2018-11-14T19:27:40Z

numpy/lib/tests/test_histograms.py


        # these should not crash
-        np.histogram([np.array([0.5]) for i in range(10)] + [.500000000000001])
-        np.histogram([np.array([0.5]) for i in range(10)] + [.5])
+        np.histogram([np.array(0.5) for i in range(10)] + [.500000000000001])


This test, as written, made no sense; there is no reason you should be allowed to mix 1-d arrays with scalars, even if they are single-element.

I think I wrote these mostly to test that they didn't blow up in a non-sensical internal crash. The thinking was that this is just doing a histogram on object arrays.

If we could commit to making bool(np.array([1]) < 0) an error, then this would no longer be allowed either.

I see. But just to be sure: is the change here fine? If we want object arrays, it might be better to just create them explicitly.

hameerabbasi

I'm +0.2 on this change. I agree that it could make a lot of sense.

Adding the axis kwarg would be nice, it would be a simple transpose operation at the end. :)

hameerabbasi · 2018-11-14T19:35:57Z

numpy/core/function_base.py

@@ -34,9 +34,9 @@ def linspace(start, stop, num=50, endpoint=True, retstep=False, dtype=None):

    Parameters
    ----------
-    start : scalar
+    start : scalar or array


Would array_like be better?

hameerabbasi · 2018-11-14T19:36:05Z

numpy/core/function_base.py

        The starting value of the sequence.
-    stop : scalar
+    stop : scalar or array


shoyer · 2018-12-01T20:19:18Z

I wrote a small PR to add dispatching to these functions: #12471

charris · 2018-12-04T14:53:57Z

The request for array_like seems reasonable. What about subtypes?

If this is intended for 1.16 it needs a release note. I'd like to branch 1.16.x soon.

And which failed with the change to linspace.

mhvk · 2018-12-04T20:33:24Z

@charris - OK, I added the array_like and also ensured things work for logspace and geomspace - the latter was a bit more of a change.

Note that I did not add an axis argument - I can do that but felt the case for it was not that strong; it is trivial also to add the swapaxes oneself.

charris · 2018-12-05T00:22:35Z

numpy/core/tests/test_function_base.py

@@ -54,6 +54,18 @@ def test_basic(self):
        y = logspace(0, 6, num=7)
        assert_array_equal(y, [1, 10, 100, 1e3, 1e4, 1e5, 1e6])

+    def test_array(self):


test_start_stop_are_arrays or some such would be a more descriptive name here and below.

It was because the test above was just called test_array_scalar - but really both are unclear, so I changed both now.

charris · 2018-12-05T00:29:59Z

Just a suggestion for a name change for the new tests.

eric-wieser · 2018-12-05T06:31:05Z

numpy/core/function_base.py

@@ -72,7 +72,8 @@ def linspace(start, stop, num=50, endpoint=True, retstep=False, dtype=None):
    samples : ndarray
        There are `num` equally spaced samples in the closed interval
        ``[start, stop]`` or the half-open interval ``[start, stop)``
-        (depending on whether `endpoint` is True or False).
+        (depending on whether `endpoint` is True or False).  If start or
+        stop are array-like, then the samples will be on the first axis.


Perhaps "will be along a new axis inserted at the beginning" would be more accurate

eric-wieser · 2018-12-05T06:32:02Z

numpy/core/function_base.py

+    start = start.astype(dt, copy=True)
+    stop = stop.astype(dt, copy=True)
+
+    out_sign = _nx.ones((start + stop).shape, dt)


Addition here is pretty wasteful to get np.broadcast(start, stop).shape

Yes, and clearer in intent too. Done.

eric-wieser · 2018-12-05T06:32:50Z

numpy/core/function_base.py

-        start = start + 0j
-        stop = stop + 0j
+    if _nx.issubdtype(dt, _nx.complexfloating):
+        all_imag = (start.real == 0.) & (stop.real == 0)


nit: spell 0. as 0 to make the two terms consistent

eric-wieser · 2018-12-05T06:34:29Z

numpy/core/function_base.py

        raise ValueError('Geometric sequence cannot include zero')

-    dt = result_type(start, stop, float(num))
+    dt = result_type(start, stop, float(num), _nx.zeros((), dtype))


Does this behave differently than just passing dtype in directly?

dtype can be None - I actually tried thinking of a quicker way.

mhvk · 2018-12-05T14:48:05Z

OK, thanks. Adopted the suggestions.

numpy/core/function_base.py

shoyer · 2018-12-05T19:13:43Z

I really do this we should add an axis argument, which could default to axis=0. People seem to have different intuitions about what this "should do" (this also came up on the mailing list discussion). The major advantage of the explicit axis argument is that it allows for clearly documenting what this function does at the call site. Also, it will result in clearer errors if you try to rely on support for array-like arguments but are actually using an old version of NumPy that doesn't support it.

Note: from an implementation perspective, you could do this just by adding np.moveaxis(result, 0, axis) at the end.

mhvk · 2018-12-05T20:16:22Z

OK, I'll do it - I was trying to avoid the extra work of also writing tests... I guess little choice but to put it at the end of the argument list?

mhvk · 2018-12-05T20:54:58Z

Now added the axis argument...

hameerabbasi

Just a minor doc question.

numpy/core/function_base.py

…stop

mhvk · 2018-12-06T18:10:37Z

@hameerabbasi - good point. I also added a versionchanged stanza so that people know when array-like start and stop were implemented.

charris · 2018-12-06T19:32:49Z

Thanks Marten.

mhvk added 01 - Enhancement component: numpy._core 25 - WIP labels Nov 14, 2018

mhvk commented Nov 14, 2018

View reviewed changes

mhvk changed the title ~~Linspace allow array~~ ENH: allow arrays for start and stop in linspace Nov 14, 2018

hameerabbasi reviewed Nov 14, 2018

View reviewed changes

shoyer mentioned this pull request Dec 1, 2018

ENH: override support for np.linspace and friends #12471

Merged

TST,BUG: fix two tests that were setup incorrectly.

bd1d6a5

And which failed with the change to linspace.

mhvk force-pushed the linspace-allow-array branch from 78cf869 to 099dda6 Compare December 4, 2018 20:31

mhvk changed the title ~~ENH: allow arrays for start and stop in linspace~~ ENH: allow arrays for start and stop in {lin,log,geom}space Dec 4, 2018

mhvk removed the 25 - WIP label Dec 4, 2018

mhvk added this to the 1.16.0 release milestone Dec 4, 2018

charris reviewed Dec 5, 2018

View reviewed changes

eric-wieser reviewed Dec 5, 2018

View reviewed changes

ENH: Allow {lin,log,geom}space start and stop to be arrays.

58ebb6a

mhvk force-pushed the linspace-allow-array branch from 099dda6 to 58ebb6a Compare December 5, 2018 14:47

charris reviewed Dec 5, 2018

View reviewed changes

numpy/core/function_base.py Outdated Show resolved Hide resolved

ENH: allow user choice of sample axis in {lin,log,geom}space.

d338490

hameerabbasi reviewed Dec 6, 2018

View reviewed changes

numpy/core/function_base.py Outdated Show resolved Hide resolved

numpy/core/function_base.py Outdated Show resolved Hide resolved

DOC: Note the version in which we start supporting array_like start, …

abf6262

…stop

charris merged commit 2970e41 into numpy:master Dec 6, 2018

mhvk deleted the linspace-allow-array branch December 6, 2018 20:54

EelcoPeacs mentioned this pull request Dec 12, 2018

Local file existence checking requires URL modules to be loaded in _datasource.py #12536

Closed

mhvk mentioned this pull request Mar 22, 2019

ENH: allow linspace to take 1D vectors as start and end #2944

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: allow arrays for start and stop in {lin,log,geom}space #12388

ENH: allow arrays for start and stop in {lin,log,geom}space #12388

mhvk commented Nov 14, 2018 •

edited

Loading

mhvk Nov 14, 2018

mhvk Nov 14, 2018

eric-wieser Dec 5, 2018

mhvk Dec 5, 2018

hameerabbasi left a comment

hameerabbasi Nov 14, 2018

hameerabbasi Nov 14, 2018

shoyer commented Dec 1, 2018

charris commented Dec 4, 2018

mhvk commented Dec 4, 2018

charris Dec 5, 2018

mhvk Dec 5, 2018

charris commented Dec 5, 2018

eric-wieser Dec 5, 2018

mhvk Dec 5, 2018

eric-wieser Dec 5, 2018

mhvk Dec 5, 2018

eric-wieser Dec 5, 2018

mhvk Dec 5, 2018

eric-wieser Dec 5, 2018

mhvk Dec 5, 2018

mhvk commented Dec 5, 2018

shoyer commented Dec 5, 2018

mhvk commented Dec 5, 2018

mhvk commented Dec 5, 2018

hameerabbasi left a comment

mhvk commented Dec 6, 2018

charris commented Dec 6, 2018

ENH: allow arrays for start and stop in {lin,log,geom}space #12388

ENH: allow arrays for start and stop in {lin,log,geom}space #12388

Conversation

mhvk commented Nov 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hameerabbasi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shoyer commented Dec 1, 2018

charris commented Dec 4, 2018

mhvk commented Dec 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Dec 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhvk commented Dec 5, 2018

shoyer commented Dec 5, 2018

mhvk commented Dec 5, 2018

mhvk commented Dec 5, 2018

hameerabbasi left a comment

Choose a reason for hiding this comment

mhvk commented Dec 6, 2018

charris commented Dec 6, 2018

mhvk commented Nov 14, 2018 •

edited

Loading