Adds keyword argument to choose faster convolution method

As mentioned in #2651, this PR adds the keyword argument method to scipy.signal.convolve to choose the choose the convolution method. method can take vales 'auto', 'fft' or 'direct'.
In scikit-image PR 1792 we chose the faster convolution method, either the direct method or with fftconvolve. We merged these changes in, and I am merging these changes upstream.
Timing comparison
In the plot below, the ratio fft_time / direct_time is plotted with a log scaling. (more detail can be found in the scikit-image PR). These plots are lifted from the scikit-image PR.

In the plot below, the fit values to this ratio are shown. If this ratio is less than 1, this PR chooses to use fftconvolve.

The complete code and data to generate these plots are in a gist. These timings were performed on a Mid-2012 Macbook Air.
Tests
Tests on my machine failed to build. However, my own test passes:
for pwr in [4, 5, 6, 7, 8, 9, 10, 11, 12]:
    for _ in range(int(1e3)):
        x = np.random.rand(2**pwr)
        h = np.random.rand((2**pwr)//10)

        y1 = convolve(x, h, method='fft')
        y2 = convolve(x, h, method='direct')

        assert np.allclose(y1, y2), "convolve should give equivalent answers"

Documentation
This PR includes documentation of this keyword argument in the docstring. I tried to follow the form of the mode keyword argument.

This looks interesting, potentially large speedups.

Tests on my machine failed to build.

They seem to pass on TravisCI without any adjustment, which is slightly surprising. Do you have the test failures? Also, did you check how much the relative precision changes for a range of input arrays?

I found that the relative precision difference was small (on the order of 1e-15).
np.random.seed(42)
pwr = 16
x = np.random.rand(2**pwr)
h = np.random.rand((2**pwr)//10)
y_fft = convolve(x, h, method='fft')
y_direct = convolve(x, h, method='direct')

error = abs(y_fft - y_direct) / y_direct

print("error std. dev = {:0.4e}".format(np.sqrt(error.var())))
print("error mean = {:0.4e}".format(error.mean()))
print("error max = {:0.4e}".format(error.max()))
print("error median = {:0.4e}".format(np.median(error)))

# prints the following output:
# error std. dev = 1.3594e-14
# error mean = 6.9658e-16
# error max = 3.0790e-12
# error median = 2.8107e-16

The tests failed to build on my machine for an unrelated issue. I ran into some issue with gcc and Homebrew.

The code LGTM, but I'll let someone more current on things SciPy take the decision to merge it.

@scottsievert do you need others to run a benchmark script to tune your algorithm? I have a 6-core desktop running Linux (self-built libs) and a 2-core laptop running Windows (Anaconda) that I could use if you want more data.

And have you looked at all at whether the use of MKL affects it? I suppose it might be a bit too fine-grained for the optimization scheme you're going for, though.

Actually @scottsievert do you have a link to the code used to generate times.p? I can only seem to find the code for doing the plot in the linked issue and gist, not the code to actually generate the times. I'd like to extend it to (and test) 1D as well as 2D/3D, and validate the existing results on my system(s).

I updated the gist to include code to generate times.p (and it also includes a link to the Jupyter notebook). I tested the mentioned times on a Macbook Air with 8GB memory and Anaconda installed (without MKL).
I only did limited testing in the skimage PR (I only ran two tests). I just wanted a very rough rule that could detect when speedups of 10x or greater where present.

Did you also look at larger images?  In my domain the scientifically relevant image sizes are typically (1-2k) x (1-2k) .

Here's the image from a speed test we did again:

The speed gains only increase for larger images and larger kernels. fftconvolve is faster for large images and large kernels -- even for a 200x200 image and 25x25 kernel, fftconvolve took roughly a factor of 2^-4 or 2^-5 as long and was much faster. We didn't test larger images because convolve took too long.

I tweaked the script and ran it on my MKL system, and the decision boundary looks good (the shortest 1D case is for a 3 sample signal, which is why I suspect it's off -- but it shouldn't really matter which version you use there I guess):

We went down a similar route on trackpy and came to the opposite conclusion (see https://github.com/soft-matter/trackpy/issues/219 and links @danielballan and @caspervdw can comment in more detail than I can).  I think the difference is that we were only using separable kernels so direct 1D convolution still won.

@tacaswell I think the above plot is saying that direct 1D convolution does win over FFT-based, since it's a plot of t_fft / t_direct (and above unity most of the instances)...?

FWIW: http://scipy-cookbook.readthedocs.org/items/ApplyFIRFilter.html
The plot is a few years old.  I haven't run that script in quite a while, so I don't know if it would look the same now.

Hmm, so maybe we need some additional testing, tuning, and/or triage in the 1D case.

I think the default method should be 'auto'.  The existing docstring only says that the function computes the convolution.  Mathematically, both the direct method and the FFT method are equivalent, so we don't really have a problem with backwards compatibility.  Of course, the results will not be exactly the same, but scipy does not promise bit-level reproducibility between versions.  'auto' is what we want in the long term; I think we can safely do that now.

Fair enough

Modified FIR script is here: https://gist.github.com/WarrenWeckesser/e2f4de588967ea01226e
I dropped lfilter from the script.  Thanks to @ewmoore, lfilter is now smart about handling the case when the a argument of lfilter is a scalar, and the results for lfilter are pretty much the same as those of np.convolve.
Here's the result when the script is run on a Macbook Pro (OS X 10.9.5), using Anaconda's packages (scipy 0.16.1, numpy 1.10.2):

Hm, interesting. Looks like these changes should be merged upstream to numpy (after changing the constant).
If bit-level reproducibility is not required, I believe the default for method should be auto, which I agree would be desired long-term.

If bit-level reproducibility is not required, I believe the default for method should be auto, which I agree would be desired long-term.

If it all matches with relative precision of 1e-15 as you said above, that's OK. Bit-level reproducibility is anyway not guaranteed - depends on OS, compilers, CPU, etc.

I've done more testing. I've found that the 1D kernel estimation tends to be inaccurate. For 1D, I had to modify the big O constant by a factor of 26 and even then it tends to break down with small inputs. I think it'd be best to add an if statement to catch these cases; maybe if kernel.ndims == 1 and kernel.size*signal.size < 5e3? I believe these small 1D cases are off because of additional overhead in the FFT function.

I have made changes to the big O constant for the 1D case. What else should be done before this PR can be merged?

It should have tests to prove that it works identically for all possible combinations of settings, dimensions, dtypes, etc.
Ideally the keyword would also be added to correlate, correlate2d and convolve2d.

Test added for the keyword argument method. I based it off the other tests which don't test for every case and only write down one case.
Agreed, ideally they would... but that would require elaborate testing plus correlate/etc is implemented in C.

Well correlation and convolution are basically the same thing: correlate(a, b) == fftconvolve(a, b[::-1]).  convolve is actually implemented using correlate, no?

I actually started doing this a while ago but didn't get very far, but here is my unfinished garbage for reference: https://github.com/scipy/scipy/compare/master...endolith:fft_correlate?expand=1
The fft method won't work for object arrays, for instance, so if method=auto and the array is a bunch of Decimals, it needs to detect that and use the direct implementation, no matter what size.
Likewise, if the input is of int type, it can be processed by fftconvolve, but the output should be converted back to same int type as the input?

Could we make it work only for numeric arrays (int, float, etc)? We should be able to handle that (luckily, convolving two int arrays should give another int array, mathematically).

I just realized it can't handle long ints and crossed out what I said above.  And yes, handling all object arrays using direct method is fine with me, even if it's an object array of floats.  It doesn't have to dissect object arrays to find out if the contents are compatible.  So if method=auto and dtype is object, use direct.  if method=fft and dtype is object, raise an exception.

Optional argument implemented for correlate.
correlate checks to see if the arrays are the right type/etc then calls convolve. This either calls fftconvolve or correlate, depending on the sizing. So a function call to correlate wouldn't be stuck in infinite loop, I added explicit method=default arguments in convolve after sizing checks are performed.

No diff could be generated. No reports for master found.

Powered by Codecov. Updated on successful CI builds.

The method optional argument has been added to correlate as well. As with convolve, it defaults to auto.
In doing this, I ran into an issue when computing the correlation of complex variables with fftconvolve. Specifically, I ran into an issue where the imaginary/real parts were flipped (i.e., errors of the form expected == -1 + 2j != 2 - 1j == observed). Quick tests indicate that I shouldn't expect these two values to be equal.

I'm not sure I understand your comment, but sorry if I mislead you, it's actually
correlate(a, b) == fftconvolve(a, conj(b)[::-1])

I forgot about the conjugation for complex signals.  Wikipedia actually conjugates and reverse the first argument, but that reverse the output for me: https://en.wikipedia.org/wiki/Cross-correlation#Properties
correlate(a, b, mode='full')
Out[24]:
array([   5. +45.j,   20. +93.j,   44.+143.j,   76.+194.j,  115.+245.j,
        100.+190.j,   80.+137.j,   56. +87.j,   29. +41.j])

fftconvolve(a, conj(b)[::-1])
Out[25]:
array([   5. +45.j,   20. +93.j,   44.+143.j,   76.+194.j,  115.+245.j,
        100.+190.j,   80.+137.j,   56. +87.j,   29. +41.j])

Update: I have implemented this method for correlate as well. I have tested both methods to ensure that two methods for convolve/correlate are np.allclose. That is, I test np.allclose(correlate(..., method='fft'), correlate(..., method='direct')) (and the same for convolve).

It should probably test equivalence with every mode setting too.
Is it necessary to skip .conj() for real input? I would think it quickly does nothing if the type is real and there's no harm in eliminating the if/then?

The tests are more complete; based off @endolith's comments, I test

all types for correlate by actual using the framework for _TestCorrelateReal
that it raises ValueError when convolve is passed two object arrays with method='fft'
that the dtypes of the input/output is equal with both methods

correlate: The _modedict check and if in1.ndim == in2.ndim == 0: case should be moved up ahead of the method checks, I think?  So invalid modes are caught before passing to convolve and correlate(4, 5) evaluates right away without wasting time checking things.
Also can you convert the slice in line 223 of the existing code to match your more readable version?
slice_obj = [slice(None, None, -1)] * len(z.shape)
z = z[slice_obj].conj()

to
reverse = [slice(None, None, -1)] * z.ndim
z = z[reverse].conj()

[x] I check for large integers. If the maximum output is >  2^52-1, I say method=direct. I check for this in correlate. I do not test that the correlation of two large integer arrays is the expected result.
[x] I have moved the if statements that check the number of dimensions/etc to before the check for method as described in https://github.com/scipy/scipy/pull/5608#issuecomment-189095242
[x] I have cleaned up the existing code and the end of correlate as described in https://github.com/scipy/scipy/pull/5608#issuecomment-189095242

More work on docstring/renaming/etc done.
I've also done timing comparisons for convolve2d with this method. In this, I find that convolve2d is faster than the minimum time required for fftconvolve or convolve.

I've looked at adding a similar if-statement to convolve2d but there are additional keyword args (fillvalue and boundary) which are handled in C (and the implementation isn't simple enough I can handle it quickly).

I would leave convolve2d/correlate2d for another PR

Turns out the output needs to play a factor when during the big O computation. After I factor that in, I get the following result:

Full set of results: http://imgur.com/a/Cxij2 (for mode in {'valid', 'full'}, <= 3 dimensions). Asymptotically, the performance of the predictor is much better than my previous prediction.

Also documentation in https://github.com/scipy/scipy/blob/master/doc/source/tutorial/signal.rst should be updated to explain that convolve automatically calls fftconvolve now.

Updated the tutorial.
I have been able to build the tutorial and see the output I expect. However, I haven't been able to build scipy (and hence the docstring docs). Can someone verify that the docstring for convolve/correlate/etc  look as expected? I followed previous style but want to verify.

I've never gotten anything to build on my machine, but the docstring formatting looks correct.    Spyder will display most of it in rendered form.

I tried to figure out the number of operations for direct convolution.  Does this seem right?

full mode:
output shape: a.shape + b.shape - 1
output size: prod(output_shape)

of multiplies: a.size * b.size

of additions: a.size * b.size - output_size

valid mode
output shape: abs(a.shape - b.shape) + 1
output size: prod(output_shape)

of multiplies: min(a.size, b.size) * output_size

of additions: min(a.size, b.size) * output_size - output_size

same mode
output shape: a.shape
output size: prod(output_shape)
...

No worries -- you want me to git pull those PRs in and resolve the merge conflicts myself?
I think waiting after the inputs are reshapes by the size parameter means the number of additions/multiplications isn't important and the speed is determined by the big O constant.

I think waiting after the inputs are reshapes by the size parameter

I don't understand what you mean by this.  Currently you're estimating time using direct_time like this:

full: volume.size * kernel.size * prod(volume.shape + kernel.shape)
same: volume.size * kernel.size * volume.shape
valid: volume.size * kernel.size * prod(volume.shape - kernel.shape + 1)

and I was trying to figure out where these came from, and came up with these instead.  So for full, the number of operations is number of muls + number of adds, approximately = 2 * a.size * b.size - prod(a.shape + b.shape - 1), for instance.

In this, I'm calculating the size of the final output. This is important for the timing information; the size of the output helps determine the speed of the calculation. In the O(N log N)/O(N^2) computation, N is the values in output.shape, a.shape and b.shape.

Mode
Output shape

full
a.shape + b.shape - 1

valid
a.shape - kernel.shape + 1

same
a.shape

I have verified all of these for 1D, and the shape for full is currently off by 1 (I've fixed it but I won't devote a commit to that).

though in reality it doesn't work out that way...
a = rand(100, 10)
b = rand(100, 10)

a.size * b.size
1000000

2 * a.size * b.size - prod(array(a.shape) + array(b.shape) - 1)
1996219

timeit convolve(a, b, 'full')
10 loops, best of 3: 93.3 ms per loop

a = rand(1000, 1)
b = rand(1000, 1)

a.size * b.size
1000000

2 * a.size * b.size - prod(array(a.shape) + array(b.shape) - 1)
1998001

timeit convolve(a, b, 'full')
10 loops, best of 3: 46.4 ms per loop

a = rand(1000, 1)
b = rand(1, 1000)

a.size * b.size
1000000

2 * a.size * b.size - prod(array(a.shape) + array(b.shape) - 1)
1000000

timeit convolve(a, b, 'full')
1 loops, best of 3: 21.9 s per loop

22 seconds??  Something's not right with _correlateND
a = rand(1000, 1)
b = rand(1, 1000)
c = convolve(a, b, 'full')

allclose(c, outer(a, b))
Out[108]: True

timeit convolve(a, b, 'full')
1 loops, best of 3: 20.6 s per loop

timeit outer(a, b)
100 loops, best of 3: 5.13 ms per loop
``

Using the calculation above, I found this predicted ratio against the actual ratio:

I updated the imgur gallery to reflect the additions for mode == same.
I believe smaller sizes (roughly N < 17) are off because of functions to prepare for the FFT.

I've spent far too long trying to rebase on top of the PRs 5904 and 5909. Can you point me to a method/guide/etc?

Well I use SmartGit on Windows, and do it like this:

pull the latest changes from "upstream" git://github.com/scipy/scipy.git
checkout convolve-method
right-click scipy and go to Log
right-click on upstream-master, click "rebase HEAD to..." and press the button
It starts to rebase then fails because of the conflicts
Go back to file view, right click on conflicted files and select Conflict Solver
Click red arrow to "Next Conflict"
Figure out which stuff should be kept from each source and edit the middle column accordingly
Save button, Close Conflict Solver, yes to "Stage" button
Press the Commit button and it asks to continue the rebase
Probably repeatedly fails again, go back to step 6 each time
Finally it succeeds and you can go through all the changes in the Log and make sure they still make sense
When happy, right-click on the branch, "Push..."  check the box for Force Pushing

I just did it while writing this, it was painful:
https://github.com/endolith/scipy/tree/convolve-method-rebase
I also squashed some commits like typo fixes on previous commits

I personally do:
1. git checkout convolve-method
2. git fetch upstream
3. git checkout -b convolve-method-bak  # make a backup
4. git push origin convolve-method-bak  # push it remotely if I'm feeling uber paranoid
5. git checkout convolve-method
6. git rebase upstream/master
<hit a conflict>
7. git mergetool  # this auto-launches "meld" for me, which is an awesome tool
8. git rebase --continue
<Repeat steps 7 and 8 for each conflict>
9. git push origin --force convolve-method

Although in your case you wouldn't want to rebase on upstream/master but rather whatever branch those PRs refer to

Smartgits conflict solver looks a lot like meld
Those prs were merged into upstream master already, which is why I rebased onto that
I think you can checkout my branch above, but with the name convolve-method, which will overwrite yours with the rebased version and then force push it to this PR. Make a backup first by checking out yours with a different branch name

Now the diff is crazy.  :)  Can you overwrite yours with my rebased one instead of merging them?  What git software are you using?

Okay, merged in.

Also think I made a mistake while rebasing ... It's testing that valid mode fails for wrong size order, but that isn't true anymore.

I didn't run into this issue.

I have tested all three mode values and <= 3 dimensions. In all cases, it looks asymptotically accurate.

It does look good. Instead of looking at ratios have you tried looking at the difference? I wonder if there is some constant value (e.g., how long it takes to set up the FFT) that could be added to the equation to make it better.
Is the [gist] (https://gist.github.com/scottsievert/01ba818e2b16fcdec34d) up-to-date? If so I might try seeing if I get the same values on my system.

[This Dropbox folder][1] is more current. It has an Jupyter notebook that performs timing and generates a graph.
Subtraction sounds like the right approach. I'll look into that next (using the same Jupyter notebook as above).

Let me know if you get anywhere with the subtraction, and then I'll see if my machine(s) produce similar results to yours. Then hopefully this will be about ready to go.

I see about the same performance in terms of prediction accuracy when I include a constant setup time. This is a harder problem (3 variables to tune instead of 1), so I use scipy.optimize.curve_fit. When I do, I see the following results:

I'm more inclined to go with the time ratio, as that's the simpler solution and looks to have similar accuracy. I also tried including a linear setup time (not constant) but saw less accurate results.

Thanks for trying. Is the dropbox up to date? I'd like to give it a spin
tomorrow to see if I get similar curves.

It is (and it's how I played around with the subtraction). Running it straight through should give you similar graphs, but be warned that it takes a while with my choice of sizes.
On Mar 22, 2016, 7:01 PM -0600, Eric<EMAIL_ADDRESS>wrote:

Thanks for trying. Is the dropbox up to date? I'd like to give it a spin
tomorrow to see if I get similar curves.
—
You are receiving this because you were mentioned.
Reply to this email directly orview it on GitHub(https://github.com/scipy/scipy/pull/5608#issuecomment-200099646)

There appears to be a small error in your scritpt -- I had to do setup=1 instead of setup=setup otherwise I got NameError: name 'setup' is not defined. (I also had to change the path from your hard-coded one, but that's expected.) Doing setup=0 didn't seem to change much. Should it?
Do I need to be on this branch to test the timing algorithm, or is being on e.g. 0.17 okay? I used 0.17 Anaconda for this test.
It only took a couple of minutes, not bad. On Linux with Anaconda/MKL, I get:

Some of those predictions for 1D are off by an order of magnitude, and don't appear to converge. Any idea what's going on? Do you think it's me being on 0.17?

Do I need to be on this branch to test the timing algorithm, or is being on e.g. 0.17 okay? I used 0.17 Anaconda for this test.

I do all my tests on 0.17 and call convolve/fftconvolve directly -- I assume the calling of functions is minimal.

Some of those predictions for 1D are off by an order of magnitude, and don't appear to converge. Any idea what's going on?

Hm... it looks like it's off by the scaling/size, or the big O constant ratio out front. I would assume that the speed of the computer appears in both big O constants and hence cancels. I never compute this or fit a curve to find the constant -- I only fit the ratio of the times, never the speed of the function itself.
I would guess this is the issue -- I'm never precisely finding the big O constant for convolve or fftconvolve. I'll do some more work to do this. That should give a more precise ratio.
Does that make sense? That's the only thing I can think of that explains the difference.

I do all my tests on 0.17

Do you use MKL or not? I am not sure how extensively convolve vs fft can make use of SIMD under the hood, but it could matter.

Does that make sense? That's the only thing I can think of that explains the difference.

I don't have any other ideas.

Looks like the issue is more basic. I timed the functions and computed the average, not the minimum. The minimum makes more sense as the computer may be busy with other tasks; it's how IPython's %timeit works, etc.
I've updated the Jupyter notebook; @Eric89GXL, can you test and see if you get close results? The script should take longer; to get a stronger minimum, I added changed the number of timing iterations in the function time. Feel free to change these.

Ahh yes, my results match yours now:

Although a couple of the decision points are still off by a bit, I'm pretty happy with this.
@endolith any other comments from you?
@stsievert can you try squashing the commits to just a few? Anything else you want to do before final reviews and merge?

@stsievert I'm trying to figure out what to do about this:
http://scipy-cookbook.readthedocs.org/items/ApplyFIRFilter.html
If that plot is still valid, then wrapping to convolve is not a great choice for the 1D case (maybe even when n is small). Do you think this can be tackled as part of this PR, or should it be left to another one?
If further optimization is left to another PR, it's probably worth at least regenerating this figure with your PR in place to see hopefully that signal.convolve does better now.

Actually it looks like @WarrenWeckesser updated the script, from a comment above:
https://gist.github.com/WarrenWeckesser/e2f4de588967ea01226e

wrapping to convolve is not a great choice for the 1D case

Looks like the plot is out of date. scipy.signal.convolve calls np.convolve in the 1D case, but this was only added by #5291 for 0.17. The speed prediction takes this into account; I compute a different constant for the 1D case.

Actually it looks like @WarrenWeckesser updated the script, from a comment above:

I'll send a pull request off for pv/ApplyFIRFilter.ipynb in the near future.

Looks like the plot is out of date.

So with this PR is signal.convolve the fastest? Have a preview of the updated plot?

Without modification:

With modification to move the decision boundary:

It makes sense that np.convolve is faster; the signal is 4 x 2**17, and the script just loops over each of the four elements. Because it's not a 1D signal, it uses the same implementation as the earlier plot.
One possible enhancement is to see when looping over each dimension with np.convolve is faster than using the current implementation. This shouldn't be too, just a division of two big O constants.
lfilter doesn't have the keyword arg for mode, so we can't use that function.

For FIR lfilter just calls np.convolve anyway.
On Sunday, March 27, 2016, Scott Sievert<EMAIL_ADDRESS>wrote:

Without modification:
[image: no_mods]
https://cloud.githubusercontent.com/assets/1320475/14066130/b4a812bc-f407-11e5-963c-b1132407a9ed.png
With modification to move the decision boundary:
[image: with_mods]
https://cloud.githubusercontent.com/assets/1320475/14066134/bdfb0018-f407-11e5-9a93-b76cc134473c.png
It makes sense that np.convolve is faster; the signal is 4 x 2**17, and
the script just loops over each of the four elements. Because it's not a 1D
signal, it uses the same implementation as the earlier plot.
One possible enhancement is to see when looping over each dimension with
np.convolve is faster than using the current implementation. This
shouldn't be too, just a division of two big O constants.
lfilter doesn't have the keyword arg for mode, so we can't use that
function.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/scipy/scipy/pull/5608#issuecomment-202093926

I could make it work for a 1D kernel function, but I don't this it's useful to generalize np.convolve to 2-D

It would be nice to be able to tell people what the fastest function is to accomplish 1D convolution / FIR filtering. Maybe the best thing to do is make lfilter smarter about FFT or direct methods instead of making convolve or correlate defer to np.correlate, since those functions seem more tailored (in docstring and code) to the ND case than the 1D case. Not sure, though.

I can do that in my PR to pv/SciPy-CookBook, where that graph originates, and I can also a graph for pure 1D convolution. But what else on this PR needs to be done?

I can do that in my PR to pv/SciPy-CookBook, where that graph originates, and I can also a graph for pure 1D convolution

That would be a great start, thanks.

But what else on this PR needs to be done?

I don't see anything from my end. @endolith do you have time to look one more time? @e-q you might also be interested since you've been making the filtering functions faster recently, too.
If nobody else chimes in, I'll take a final look and merge in a couple of days. If I forget feel free to remind me.

Well one thing is that the function has become pretty convoluted (:sunglasses:).  There's a conditional spanning 3 lines, for instance.  Things could be factored out into their own functions or variable names for readability.  Something like this:
if method == 'fft' and not _numeric_arrays(volume, kernel):
    raise ValueError('convolve can only ...

if method == 'auto':
    method = 'fft' if _fft_suitable(volume, kernel, mode) else 'direct'

if method == 'fft':
    out = fftconvolve(volume, kernel, mode=mode)
    ...

and then put all the big o constant code inside _fft_suitable, but also break that up into names for readability:
def _numeric_arrays(a1, a2):
    return a1.dtype.kind in 'buifc' and a2.dtype.kind in 'buifc'

def _fft_suitable(volume, kernel, mode):
    ...
    big o code
    ...
    faster = fft_time < direct_time
    numeric_inputs = _numeric_arrays(volume, kernel)
    complex256_input = volume.dtype == 'complex256' or kernel.dtype == 'complex256'

    return faster and numeric_inputs and not complex256_input

in1.ndim == in2.ndim == 1 and (in1.size >= in2.size or mode != 'same') and
volume.ndim == kernel.ndim == 1 and (volume.size >= kernel.size or mode != 'same') could both become _np_convolve_ok(in1, in2, mode) and put the detailed comments in the function's docstring instead of inline
# use faster 1d numpy.convolve if possible
if _np_convolve_ok(volume, kernel, mode):
    return np.convolve(volume, kernel, mode)

"reverse in all dimensions and conjugate" is also used 3 times and could be a function
def _reverse_and_conj(a):
    """
    Reverse array `a` in all dimensions and complex conjugate
    """
    reverse = [slice(None, None, -1)] * a.ndim
    return a[reverse].conj()

...

if swapped_inputs:
    # Undo the effect of swapping inputs
    z = _reverse_and_conj(z)

etc
also out_shape calculates 3 things even though it will only ever use one of them

on my computer
convolve(ones(100), ones(100), method='fft')
Traceback (most recent call last):

...
    and volume.dtype != 'complex256' and kernel.dtype != 'complex256':

TypeError: data type "complex256" not understood

I guess checking equality with a dtype doesn't work if that dtype isn't supported by the system?
dtype('float64') == 'float64'
Out[55]: True

dtype('float64') == 'f8'
Out[65]: True

dtype('float64') == 'nonsense'
Traceback (most recent call last):

  File "<ipython-input-56-7fcf02d1737f>", line 1, in <module>
    dtype('float64') == 'nonsense'

TypeError: data type "nonsense" not understood

dtype('float64').name == 'nonsense'
Out[59]: False

2 examples of it choosing the slower path:
a = rand(1000000)
b = rand(4)

timeit convolve(a, b, 'full', method='direct')
100 loops, best of 3: 7.27 ms per loop

timeit convolve(a, b, 'full', method='fft')
1 loop, best of 3: 252 ms per loop

timeit convolve(a, b, 'full', method='auto')
1 loop, best of 3: 217 ms per loop

a = rand(1000000)
b = rand(1000000)

timeit convolve(a, b, 'valid', method='direct')
100 loops, best of 3: 9.96 ms per loop

timeit convolve(a, b, 'valid', method='fft')
1 loop, best of 3: 445 ms per loop

timeit convolve(a, b, 'valid', method='auto')
1 loop, best of 3: 472 ms per loop

I'm not getting the same speed differences.

When I run ApplyFIR.ipynb (in the dropbox folder) I get this graph:

Yeah it works for 10**4 but not 10**5.
my computer:
a = np.random.rand(10**4)
b = np.random.rand(10**4)

timeit convolve(a, b, 'valid', method='direct')
The slowest run took 56.35 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 48.5 µs per loop

timeit convolve(a, b, 'valid', method='fft')
100 loops, best of 3: 2.74 ms per loop

timeit convolve(a, b, 'valid', method='auto')
The slowest run took 79.91 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 122 µs per loop

a = np.random.rand(10**5)
b = np.random.rand(10**5)

timeit convolve(a, b, 'valid', method='direct')
The slowest run took 39.32 times longer than the fastest. This could mean that an intermediate result is being cached.
1000 loops, best of 3: 253 µs per loop

timeit convolve(a, b, 'valid', method='fft')
10 loops, best of 3: 32.3 ms per loop

timeit convolve(a, b, 'valid', method='auto')
10 loops, best of 3: 44.2 ms per loop

and also on sagemath cloud:
https://cloud.sagemath.com/projects/14f4c48e-fece-46bd-ac2b-94d4c24f3f2b/files/2016-04-07-164023.html

Note that @endolith's example is using the mode='valid'. I can reproduce this for length 10**5 inputs.
In [10]: foo = np.random.randn(10**5)

In [11]: bar = np.random.randn(10**5)

In [12]: %timeit baz = sig.convolve(foo, bar, 'valid', method='direct')
The slowest run took 4.67 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 139 µs per loop

In [13]: %timeit baz = sig.convolve(foo, bar, 'valid', method='fft')
100 loops, best of 3: 12.2 ms per loop

In [14]: %timeit baz = sig.convolve(foo, bar, 'valid', method='auto')
100 loops, best of 3: 12.2 ms per loop

Whereas mode='full' works fine
In [15]: %timeit baz = sig.convolve(foo, bar, 'full', method='direct')
1 loop, best of 3: 4.8 s per loop

In [16]: %timeit baz = sig.convolve(foo, bar, 'full', method='fft')
100 loops, best of 3: 11.8 ms per loop

In [17]: %timeit baz = sig.convolve(foo, bar, 'full', method='auto')
100 loops, best of 3: 13.8 ms per loop

This is great stuff.
Personally, I think it could be useful for _fftconv_faster or something like it to be a public function. Specifically, I have a use case where it would be nice to just find out the fastest method for a given size once, instead of testing the inputs on each call.
Other than the choice for mode='valid' issue above, this is looking good to me.

I've looked into this, and it turns out that the reason that under mode=valid, convolving two equal sized vectors results in a single scalar output. The calculation of which method is faster works best under large n, meaning that for small sized outputs it is inaccurate.
I'm currently investigating ways to get the prediction more accurate.
In [7]: convolve(rand(100), rand(100), 'valid').shape
Out[7]: (1,)

Yeah that's why I was trying to figure out the exact number of operations,  but then https://github.com/scipy/scipy/issues/5967I threw me off

I've looked more into this issue, and have a prototype decision rule. In the graphs below, I show the actual time ratios by timing both methods. The red area corresponds to when the direct setting is faster, the blue corresponds to when fft is faster. The y-axis is the difference in signal length and corresponding to the timing of two signals of the length n and n - k where n and k are the values on the x and y axis respectively.

all results up to 6 dimensions: http://imgur.com/a/Mxw1w

After doing this, I'm inclined to go with a decision rule that only looks at the difference in signal length. In my most recent commit, I used the decision rule
        shape_diff = np.prod([n - k for n, k in zip(x.shape, h.shape)])
        # this decision was calculated with a single dimension
        shape_diff = np.power(np.abs(shape_diff), 1 / x.ndim)
        if (x.ndim == 1 and shape_diff < 100) or \
           (x.ndim < 6 and shape_diff <= 1) or (shape_diff == 0):
            return False

I tested in N-D with a n-dimensional cube (n coeffs in every dimension). My generalization of this works numerically for the cases I tested (i.e., if in 3D and am convolving a 5x5x5 cube with a 3x3x3 cube, ((5-3)^3) ** (1/3) = 2.

Still not working on my machine because of the complex256 thing

Here's another issue:
convolve([1], [2**10])
Out[70]: array([1024])

convolve([1], [2**20])
Out[71]: array([1048576])

convolve([1], [2**30])
Out[72]: array([1073741824])

convolve([1], [2**40])   # overflow
Out[73]: array([-2147483648])

convolve([1], [2**50])   # overflow
Out[74]: array([-2147483648])

convolve([1], [2**60])
Out[75]: array([1152921504606846976], dtype=int64)

convolve([1], [2**70])
Out[76]: array([1180591620717411303424L], dtype=object)

if max_value > 2**52 - 1 might need to be platform-specific?  on my system:
sys.maxint
Out[77]:<PHONE_NUMBER>

np.iinfo('i').max
Out[78]:<PHONE_NUMBER>

On my machine:
In [99]: fftconvolve(array([1], dtype='int64'), array([1], dtype='int64')).dtype
Out[99]: dtype('float64')

In [100]: fftconvolve(array([1], dtype='int'), array([1], dtype='int')).dtype
Out[100]: dtype('float64')

In [104]: fftconvolve(array([1], dtype='uint'), array([11], dtype='uint')).dtype
Out[104]: dtype('float64')

In [108]: convolve(array([1], dtype='int'), array([2**20], dtype='int')).dtype
Out[108]: dtype('int64')

It looks like fftconvolve always returns a float64 (on my machine) and convolve mirrors the input dtype. Looking at the source, it looks like fftconvolve uses dtype complex (source) and dtype float (source). I made the change to 2**np.finfo('float').nmant - 1 to make it more clear.

Fftpack doesn't have long doubles. Float64, float32, complex128, and
complex64 are provided. Long doubles should probably go via the direct
method with auto since otherwise they will be silently downcast.
On Wednesday, April 13, 2016, Scott Sievert<EMAIL_ADDRESS>wrote:

On my machine:
In [99]: fftconvolve(array([1], dtype='int64'), array([1], dtype='int64')).dtype
Out[99]: dtype('float64')
In [100]: fftconvolve(array([1], dtype='int'), array([1], dtype='int')).dtype
Out[100]: dtype('float64')
In [104]: fftconvolve(array([1], dtype='uint'), array([11], dtype='uint')).dtype
Out[104]: dtype('float64')
In [108]: convolve(array([1], dtype='int'), array([2**20], dtype='int')).dtype
Out[108]: dtype('int64')
It looks like fftconvolve always returns a float64 (on my machine) and
convolve mirrors the input dtype. Looking at the source, it looks like
fftconvolve uses dtype complex (source
https://github.com/numpy/numpy/blob/v1.10.1/numpy/fft/fftpack.py#L447)
and dtype float (source
https://github.com/numpy/numpy/blob/v1.10.1/numpy/fft/fftpack.py#L359).
I made the change to 2**np.finfo('float').nmant - 1 to make it more clear.
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/scipy/scipy/pull/5608#issuecomment-209217676

convolve([4], [5], 'valid', method='fft') works now, but convolve([1], [2**50]) still doesn't.
you should be writing tests for every bug that you fix, by the way, to make sure no one re-introduces them in the future
a = convolve([4], [5], 'valid', method='fft')
assert_array_equal(a, [20])

for n in range(10, 80, 10):
    assert_equal(convolve([1], [2**n]), 2**n)

On every dtype and mode, I test to make sure the arrays are allclose and that convolve returns the same dtype. On this, I had to change the relative tolerance in np.allclose on line 168 for two dtypes (np.complex64, np.float32) -- is this kosher?
My tests for convolve([1], [2**50]) pass. My tests for convolve([2**50], [2**50]) don't pass and I've filed a bug report (it doesn't return the correct result in the existing implementation either; it gets caught in integer overflow).

I'd expect float32 to have a different atol/rtol compared to float64 so that should be fine

Okay, the tests are more complete.
For convolve, under every dtype (pretty much, not void or str) and every mode, I test that

the results are all close. The relative tolerance for complex64 and float32 have been increased from the default of 1e-7 to 2e-5.
the dtypes between the direct and fft method are the same (note: this does not test that the input dtype is equal to the output dtype. That relies on correlate and is in the correlate test).
that it raises a ValueError when the dtype not numeric and method=fft. I also test that it raises ValueError when passed to Decimal arrays.
When two large integers are convolved, the output is the same as the direct method (the direct method doesn't always return the correct result; I have filed another bug report about this). Additionally, when the number in the mantissa is less than 50, I test to make sure that the results are what they're supposed to be (i.e., convolve([2**n], [2**n]) == 2**(2*n)))

I haven't made any modifications to the tests for correlate. My previous implementation tested that for the given dtype,

the correlation under the two methods fft and direct are array_almost_equal
the dtypes under both methods are the same and equal to the input dtype
that an array of Decimals raises a ValueError

Sounds good to me. Are there remaining issues to be tackled, or is this good to go from your end?

It's good to go on my end -- there aren't any issues I can see.
I do want to have another crack at predicting which function is faster. Functional invariance might play a role and introducing some wiggle factors for small inputs might help.

I made some changes in https://github.com/endolith/scipy/tree/convolve_changes

Did someone look at how using pyFFTW affects the results?
Thanks for the nice work!

Well pyfftw could provide its own _fftconv_faster along with scipy_fftpack?  I don't know if it supports complex256, that should be determined by a replaceable test function, too.

Oof, the end of the semester is busy. I've now merged in the changes @endolith has made and have tested those changes, resulting in the graph below.
While testing, I ran into an issue where convolve was always took method=direct for the multidimensional tests and after I fixed this bug, I couldn't reproduce it (i.e., I don't know what I changed). I believe it has something to do with mode=valid, but I'm not certain. Can someone else speed test these results? Also, should I include tests that verify these speed ups exist?

I won't have time to run it until next week. @endolith do you have time to cross-check?

I'm running some tests and made minor improvements for small N, but I don't have real internet access for a few more days.

How did you improve small N?
Also, it sounds like you're at PyCon? I'm also here and can meet up over the sprints if you'd like.

Ok I'm home now.  No, I've never been to PyCon or anything like that, just out of town.
The direct convolve for small N only takes ~10 μs, so the calculations for auto slow it down by 10-30 times.
One thing is improving the speed of fft_time, using generator and built-in sum and math.log instead of vectorized versions, since the shape "vector" will rarely have more than 3 elements:
x = randn(3, 4, 5)
h = randn(5, 3, 3)
out_shape = [n + k - 1 for n, k in zip(x.shape, h.shape)]

timeit fft_time = np.sum([n * np.log(n) for n in (x.shape + h.shape + tuple(out_shape))])
10000 loops, best of 3: 57.7 µs per loop

timeit fft_time = sum(n * math.log(n) for n in (x.shape + h.shape + tuple(out_shape)))
100000 loops, best of 3: 7.65 µs per loop

Before:

After:

Also moving the shape_diff calculations inside the elif mode == 'valid' so that it doesn't calculate out_shape unnecessarily:
Before:

After:

Then I had it make maps of the relative timing for the x.size vs h.size plane (took forever) and looked for simpler checks for small N.  These work on my machine (Python 2.7.11 |Anaconda custom (64-bit)| (default, Feb 16 2016, 09:58:36) [MSC v.1500 64 bit (AMD64)]), but maybe different on others.  Red means fft is faster, blue means direct is faster, white is the boundary:

            if h.size <= 90 or x.size <= 90:  # orange lines
                return False
            if x.size*h.size < 600**2:  # green line
                return False

            if h.size <= 90 or x.size <= 90:  # orange lines
                return False
            if x.size <= 1600 and h.size <= 1600:  # cyan box
                return False
            if x.size == h.size:  # purple diagonal
                return False
            if x.size*h.size < 800**2:  # green line
                return False

            if h.size <= 100 or x.size <= 6:  # orange lines
                return False
            if x.size*h.size < 100**2:  # green line
                return False

(Note that there's a jump around 32768.  I don't know why.)
Example of improvement from these quick checks:
Before:

After:

These are all 1D.  Also did some 2D tests but only of one variable:

This looks like it's choosing fft incorrectly at high N:

Decision boundary is a little off:

A lot of overhead at small N:

Decision boundary is way off:

I ran the 1D test on sagemath cloud and it doesn't have the same boundary, for instance:

So these might need some tweaking.  I think it will never be exact on every system, though.
Also I was thinking that choose_conv_method could be a public function, so you can call it once at the start of processing a given size, and then know which method to use for subsequent convolve calls of the same size with no overhead.  It could even have the option to try both methods and see which is faster?  Though convolve with auto mode should still guess as quickly as possible.

Hm... interesting, thanks. I've used scipy's curve_fit to minimize the error between our expected value from computation complexity and the observed time ratio.
In this, the median error for all modes is about 0.3, meaning that this function tends to choose the correct function when they're speed is within a factor of 10^0.3 = 2 times faster/slower.
While doing this, I use the log of the ratio; in the graphs below, a value of 3 means that log10 t_fft / t_direct = 3, or the fft method is 10^3 times slower.
I will work on making choose_conv_method public soon, and I can look into adding weights to the least squares to emphasize ratios closer to 1.

The full set of results are on Dropbox and include images of ratio heat maps for 1D and timing graphs 3 modes x 2 dimensions: https://www.dropbox.com/sh/6pcp6skkc1lqfgk/AAC-krHrB3K9P9SK6dNwnG-ca?dl=0

Also, here's graph that empirically show how much slower method=auto is versus the fastest of method=fft or method=direct.

Full set of results are in the dropbox folder, direct link here: https://www.dropbox.com/sh/bdlifki77wvdsxm/AABFJEyM9rNtB0BRGhfhIsr6a?dl=0
In these results we see that even in the worst case, it never gets worse than factor of 10 off  (in that case,  method=auto only about 1 millisecond). I plan to do some more work to reduce this.

I've corrected the issue where I saw large timing errors for mode=same and ndim=1 -- now, the prediction accuracy is pretty good. For this case, I see a much more reasonable graph (it's not ~10x slower):

(full set of regenerated results available on dropbox https://www.dropbox.com/sh/r495glutlj710cs/AADCxWY64Czgjna8BKtVuNM-a?dl=0)
For mode=same, I've generated two sets of timings for both m <= n and m > n in this plot. I have results for this "training data" on m <= n and m > n. Running the empirical timings for this, I see the plots below:

h = rand(400)
h = rand(950)

We can see from this plot that there are speedups when m <= n (not m > n) under mode=same. At first, I tried to swap the two inputs but I ran into a wall: under mode=same, fftconvolve returns an array the same size as the first input while np.convolve returns an array the same size as the largest input. I was going to try and change this but didn't want to introduce backwards-compatibility issues.

fftconvolve returns an array the same size as the first input while np.convolve returns an array the same size as the largest input. I was going to try and change this but didn't want to introduce backwards-compatibility issues.

Yeah that's why I had the special case for same mode in the numpy "fastpath".  I think that has to stay the way it is.
Also, this is never going to be perfect, so when is it good enough to merge?  It can always be improved in other PRs.  If it generally does a good job of choosing the faster path without slowing things down, and the API is what we want, can it be merged?

If it generally does a good job of choosing the faster path without slowing things down, and the API is what we want, can it be merged?

+1 for merge if it's in that state. @endolith are you happy with it? @stsievert is there anything else you wanted to do?

@stsievert you may want to bunch some commits and push them to this PR at once, instead of pushing commits one by one. otherwise you're triggering CI for each commit, which is a bit much.

is there anything else you wanted to do?

@Eric89GXL My most recent commits made the choose_conv_method public and implemented tests, the last remaining task. This function should get reviewed before this gets merged.
I made a change to choose_conv_method to raise a ValueError if the given method didn't support input (e.g., dtype=complex256 and method=fft). I also included a try_ optional parameter to time the both methods.
In the tests for this function, I test everything, minus the try_ parameter -– I don't want to introduce anything non-deterministic (or any thing left up to chance) in the tests.

can it be merged?

After review of choose_conv_method and the tests, I see no reason it can't be merged.

In the tests for this function, I test everything, minus the try_ parameter -– I don't want to introduce anything non-deterministic (or any thing left up to chance) in the tests.

You could still test it. Just make sure it gives the expected output form, even if you don'

...t test the values themselves explicitly.

Can you rebase on latest master? I suspect your docstrings are not going to render nicely without some reformatting, and CircleCI will build the doc for us to find out.

I've updated choose_conv_method. I've ripped out the ValueErrors, which were raised because I was worried about fftconvolve not working with non-numeric inputs... fftconvolve raise an error if it encounters one.
Here's part of the docstring for convolve:

By default, convolve and correlate use method=auto which uses choose_conv_method to choose the method that is fastest using pre-computed values (pre-computed values by default; can perform timing with keyword argument). Because fftconvolve relies on floating point numbers, there are certain constraints that may force method=direct (more detail in choose_conv_method docstring). If method=fft is specified, fftconvolve will be called and choose_conv_method will not be.

I have built the docs, and they seem to be formatted correctly (other functions are linked, etc). How can I use circle to see the (more official) generated docs?

I have built the docs, and they seem to be formatted correctly (other functions are linked, etc). How can I use circle to see the (more official) generated docs?

You follow the link "Details" to circleci, then click there on the "Artifacts" tab, then you browse to scipy/doc/build/html-scipyorg/index.html

Sorry I screwed up sum_builtin.  It would prevent future such screw-ups to do from __builtin__ import sum as sum_builtin instead.

Re: sum_builtin
It would be much cleaner to change all occurrences of the use of numpy's sum to np.sum, so it wouldn't be necessary to mess around with the name of the builtin sum.    from __builtin__ import sum as sum_builtin smells bad.
(Sorry to jump in with just some code bikeshedding.  So let me add a big "thanks!" to y'all for the thorough research and hard work you're putting into this enhancement.)

Yeah I did it that way previously but it got lost somewhere :)
@stsievert  do you agree?
I'm adding to the docs right now

I would agree that np.sum is more clear.
...maybe a future optimization is to do the same optimization we did here, but with np.sum instead.
On Jun 20, 2016, 8:00 PM -0500<EMAIL_ADDRESS>wrote:

Yeah I did it that way previously but it got lost somewhere :)
@stsievert(https://github.com/stsievert)do you agree?
I'm adding to the docs right now
—
You are receiving this because you were mentioned.
Reply to this email directly,view it on GitHub(https://github.com/scipy/scipy/pull/5608#issuecomment-227313155), ormute the thread(https://github.com/notifications/unsubscribe/ABQmGxLNLgr2r2SXJfIFoCmMlBpH_3lpks5qNzfKgaJpZM4G1Vcl).

ok I'm going to force-push the np.sum

Ok I've looked through it all and made some small changes and it looks good to me, except for:

The complex256 rejection should really happen even if measure=True, right?  so those checks should be before the measurement?  But then they don't return times, so I wasn't sure what to do..

TODO later:

timeit_fast was intended for roughly testing hundreds of cases, so it might be good to make the time limit longer than 5 ms for this one-off measurement?
The swapped_inputs logic is ultimately a workaround for buggy _correlateND and could be
eliminated if that were ever improved.
there are no assert_raises tests for convolve or correlate?  one raises for the other if inputs are different dimensions, for instance.

Hmm... I think we should leave the measure block at the start of choose_conv_methof, before for any checks. If I was a user and changed the default, I'd like that to take effect.

Well if the array is complex256 and measure=True, choose_conv_method will fail, right?

Hm, good point. That case should go above, but what should we do about the two other tests? fftconvolve([Decimal(3)]*6, [Decimal(3)]*4) doesn’t raise any errors, and neither does the integer precision case. I've chosen a middle ground where those cases go below measure.
I’d like to see this PR in v0.18 -- is this still possible?

and you're ok with it not returning the times in that case?

Right. It should. It'd be a lot simpler to have those two cases separate, (unless we want to make two functions). We could omit measure entirely, but I don't think this issue is worth it.
I have pushed a commit that represents this, and explains some of the rationale in the documentation.

I'm not getting the same results for choose_conv_method as %timeit:
choose_conv_method(mic_sigs[0], mic_sigs[1], mode='valid', measure=True)
Out[95]: ('direct', {'direct': 3925.580464419909, 'fft':<PHONE_NUMBER>172831})

%timeit correlate(mic_sigs[0], mic_sigs[1], mode='valid', method='direct')
1000 loops, best of 3: 1.49 ms per loop

%timeit correlate(mic_sigs[0], mic_sigs[1], mode='valid', method='fft')
1 loop, best of 3: 238 ms per loop

238 ms vs 223 ms is pretty close, but 1.5 ms vs 3.9 ms is not.  I'm not sure why.
Also the tolist() is dropping the dtype:
volume = array([1, 2, 3], float16)

exec("x = array({})".format(volume.tolist()))

x.dtype
Out[94]: dtype('float64')

timeit can take a callable instead:
        for method in ['fft', 'direct']:
            times[method] = _timeit_fast(lambda: convolve(in1, in2, mode=mode,
                                                          method=method))

I've resolved these issues.

I've also documented what time unit times is in. I changed it to return from _timeit to be in seconds, chosen because it's an SI unit.

Ok I made some small changes.  I don't see any other issues.

Can we trigger another travis CI build? The test for Python 3.5 failed, but all other tests passed.

Are we ready to merge this PR in? I know I'm ready, I don't see many other areas this PR can be improved.

Should I change v0.18 to v0.19?

@endolith are you happy now?
@stsievert yes please change to 0.19

@Eric89GXL Yes it looks fine to me.

Other than my nitpicks LGTM. Can you address my comments and then squash commits (maybe to the 11 commits that would keep your and @endolith's commits separate)? Thanks for sticking with this one, seems like it has been a productive discussion.

@Eric89GXL I've addressed your comments and squashed down to 12 commits.

Anybody else want to have a look? If not, I'll merge in a few days.

There's a few issues on 32-bit systems:
======================================================================
ERROR: test_convolve_method (test_signaltools.TestConvolve)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rgommers/Code/scipy/scipy/signal/tests/test_signaltools.py", line 168, in test_convolve_method
    if x.dtype.kind not in 'buifc' or dtype == np.complex256:
AttributeError: 'module' object has no attribute 'complex256'

======================================================================
ERROR: test_rank3 (test_signaltools.TestCorrelateComplex192)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rgommers/Code/scipy/scipy/signal/tests/test_signaltools.py", line 1278, in test_rank3
    y = correlate(a, b, 'full')
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 217, in correlate
    return convolve(in1, _reverse_and_conj(in2), mode, method)
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 773, in convolve
    out = fftconvolve(volume, kernel, mode=mode)
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 391, in fftconvolve
    sp1 = fftpack.fftn(in1, fshape)
  File "/home/rgommers/Code/scipy/scipy/fftpack/basic.py", line 596, in fftn
    return _raw_fftn_dispatch(x, shape, axes, overwrite_x, 1)
  File "/home/rgommers/Code/scipy/scipy/fftpack/basic.py", line 605, in _raw_fftn_dispatch
    raise ValueError("type %s is not supported" % tmp.dtype)
ValueError: type complex192 is not supported

======================================================================
ERROR: test_rank3 (test_signaltools.TestCorrelateComplex192)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rgommers/Code/scipy/scipy/signal/tests/test_signaltools.py", line 1278, in test_rank3
    y = correlate(a, b, 'full')
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 217, in correlate
    return convolve(in1, _reverse_and_conj(in2), mode, method)
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 773, in convolve
    out = fftconvolve(volume, kernel, mode=mode)
  File "/home/rgommers/Code/scipy/scipy/signal/signaltools.py", line 391, in fftconvolve
    sp1 = fftpack.fftn(in1, fshape)
  File "/home/rgommers/Code/scipy/scipy/fftpack/basic.py", line 596, in fftn
    return _raw_fftn_dispatch(x, shape, axes, overwrite_x, 1)
  File "/home/rgommers/Code/scipy/scipy/fftpack/basic.py", line 605, in _raw_fftn_dispatch
    raise ValueError("type %s is not supported" % tmp.dtype)
ValueError: type complex192 is not supported

======================================================================
ERROR: test_signaltools.test_choose_conv_method
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rgommers/.local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/rgommers/Code/scipy/scipy/signal/tests/test_signaltools.py", line 1547, in test_choose_conv_method
    x = np.ones(n, dtype='complex256')
  File "/home/rgommers/.local/lib/python2.7/site-packages/numpy/core/numeric.py", line 190, in ones
    a = empty(shape, dtype, order)
TypeError: data type "complex256" not understood

----------------------------------------------------------------------

complex256 isn't available there. Looks like the same problem in multiple places, so factoring out a simple test method to skip over complex192/complex256 would be useful.
This is the output for me:
In [1]: np.sctypes.items()
Out[1]:
[('int', [numpy.int8, numpy.int16, numpy.int32, numpy.int32, numpy.int64]),
 ('float', [numpy.float16, numpy.float32, numpy.float64, numpy.float96]),
 ('uint',
  [numpy.uint8, numpy.uint16, numpy.uint32, numpy.uint32, numpy.uint64]),
 ('complex', [numpy.complex64, numpy.complex128, numpy.complex192]),
 ('others', [bool, object, str, unicode, numpy.void])]

@rgommers has anyone attempted to get a 32-bit build going on Travis-CI? I'm not sure how it could be done, but it would be nice to have it to lighten the load on RGommers-CI.

This does it for numpy, I've just been too lazy to look at it for scipy: https://github.com/numpy/numpy/blob/master/tools/travis-test.sh#L169

@rgommers I've resolved those issues and the tests on 64-bit systems pass. Can you verify that the checks pass 32-bit systems?

That fixed it, thanks @stsievert

Now that @rgommers is happy, I'll merge later today since it looks like all remaining comments have been addressed

Thanks @stsievert!

In at last:) Thanks @stsievert, @endolith and everyone else who helped.

Note that some of the benchmarks/signal.py benchmark results got  worse
--- there's probably a tradeoff here, or the decision criteria could be
fine-tuned:
https://pv.github.io/scipy-bench/index.html#regressions?sort=1&dir=desc

@pv  I would guess that most of the tests are small N, and would benefit from the small N shortcuts that we didn't put in _fftconv_faster yet.


ossfuzz.sh: Opt out of shift sanitizer

Shift sanitizer flags a few failures that prevent further fuzzing with other undefined behavior sanitizers enabled.
Disable shift sanitizer till the library is updated to avoid these undefined shift sanitizer issues.

cc: @ram-mohan


gyp ERR! Building erlpack

Node Version: v17.8.0
Yarn Version: v1.22.18
NPM Version: v8.8.0
Platform:

OS Name:                   Microsoft Windows 11 Pro Insider Preview
OS Version:                10.0.25115 N/A Build 25115
System Type:               x64-based PC

Compiler: MSBuild Version - <IP_ADDRESS>09
Module: Erlpack

Verbose output (from npm or node-gyp):
  | ~\Desktop\Dev\Youtube Music  ❯  yarn
yarn install v1.22.18
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning Workspaces can only be enabled in private projects.
[4/4] Building fresh packages...
[1/5] ⠈ @discordjs/opus
[-/5] ⠈ waiting...
[3/5] ⠈ utf-8-validate
[4/5] ⠈ erlpack
error C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments:
Directory: C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack
Output:
C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack>if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "C:\Users\callu\AppData\Roaming\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild )
gyp info it worked if it ends with ok
gyp info using<EMAIL_ADDRESS>gyp info using<EMAIL_ADDRESS>| win32 | x64
gyp info find Python using Python version 3.9.13 found at "C:\Users\callu\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\python.exe"
gyp info find VS using VS2022 (17.1.32421.90) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Users\callu\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\python.exe
gyp info spawn args [
gyp info spawn args   'C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\AppData\\Local\\node-gyp\\Cache\\17.8.0\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\callu\\AppData\\Local\\node-gyp\\Cache\\17.8.0',
gyp info spawn args   '-Dnode_gyp_dir=C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\\\Users\\\\callu\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\17.8.0\\\\<(target_arch)\\\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack\\build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Current\Bin\MSBuild.exe
gyp info spawn args [
gyp info spawn args   'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64'
gyp info spawn args ]
Building the projects in this solution one at a time. To enable parallel build, please add the "-m" switch.
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets(67,5): error MSB8020: The build tools for v143 (Platform Toolset = 'v143') cannot be found. To build using the v143 build tools, please install v143 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Current\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\callu\AppData\Roaming\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (node:events:527:28)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)
gyp ERR! System Windows_NT 10.0.25115
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack

Seems like both VS2022 and VS2017 are installed and the build process is trying to use both.

gyp info find VS using VS2022 (17.1.32421.90) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools"

Building the projects in this solution one at a time. To enable parallel build, please add the "-m" switch.
C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets(67,5):
error MSB8020: The build tools for v143 (Platform Toolset = 'v143') cannot be found.
To build using the v143 build tools, please install v143 build tools.
Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or
right-click the solution, and then selecting "Retarget solution".
[C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj]

Good spot, is there any method of forcing the build process to use only one of the versions while keeping both versions installed, or do I even need both and should remove VS2017?

Sorry but I am not a Windows user.

If anyone else can assist, please do.
I updated the yarn and npm config to use msvs_version 2022, uninstalled the 2017 version and tried again. Cleared cache, node_modules etc.

What is in the log?

error C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments:
Directory: C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack
Output:
C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack>if not defined npm_config_node_gyp (node "C:\Program Files\nodejs\node_modules\npm\bin\node-gyp-bin\\..\..\node_modules\node-gyp\bin\node-gyp.js" rebuild )  else (node "C:\Users\callu\AppData\Roaming\npm\node_modules\node-gyp\bin\node-gyp.js" rebuild )
gyp info it worked if it ends with ok
gyp info using<EMAIL_ADDRESS>gyp info using<EMAIL_ADDRESS>| win32 | x64
gyp info find Python using Python version 3.9.13 found at "C:\Users\callu\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\python.exe"
gyp info find VS using VS2022 (17.2.32519.379) found at:
gyp info find VS "C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools"
gyp info find VS run with --verbose for detailed information
gyp info spawn C:\Users\callu\AppData\Local\Microsoft\WindowsApps\PythonSoftwareFoundation.Python.3.9_qbz5n2kfra8p0\python.exe
gyp info spawn args [
gyp info spawn args   'C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\gyp\\gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'msvs',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack\\build\\config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   'C:\\Users\\callu\\AppData\\Local\\node-gyp\\Cache\\17.8.0\\include\\node\\common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=C:\\Users\\callu\\AppData\\Local\\node-gyp\\Cache\\17.8.0',
gyp info spawn args   '-Dnode_gyp_dir=C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp',
gyp info spawn args   '-Dnode_lib_file=C:\\\\Users\\\\callu\\\\AppData\\\\Local\\\\node-gyp\\\\Cache\\\\17.8.0\\\\<(target_arch)\\\\node.lib',
gyp info spawn args   '-Dmodule_root_dir=C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'C:\\Users\\callu\\Desktop\\Dev\\Youtube Music\\node_modules\\erlpack\\build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Current\Bin\MSBuild.exe
gyp info spawn args [
gyp info spawn args   'build/binding.sln',
gyp info spawn args   '/clp:Verbosity=minimal',
gyp info spawn args   '/nologo',
gyp info spawn args   '/p:Configuration=Release;Platform=x64'
gyp info spawn args ]
Building the projects in this solution one at a time. To enable parallel build, please add the "-m" switch.
C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj(21,3): error MSB4019: The imported project "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Default.props" was not found. Confirm that the expression in the Import declaration "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\\Microsoft.Cpp.Default.props" is correct, and that the file exists on disk.
gyp ERR! build error
gyp ERR! stack Error: `C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Current\Bin\MSBuild.exe` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onExit (C:\Users\callu\AppData\Roaming\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (node:events:527:28)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)
gyp ERR! System Windows_NT 10.0.25115
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\Users\\callu\\AppData\\Roaming\\npm\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild"
gyp ERR! cwd C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack

I fail to see why it still decides to try use the 2017 version when it is configured to use 2022

Is 2017 in the file C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj ?

Is 2017 in the file C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj ?

No, it is not.

Anyone have any ideas?

Having a look at your error it seems you might be having an issue with either the VCTargetsPath being set as an Environment Variable (user/system, not your project) and it's overwriting the inbuilt one....
Or worse the inbuilt one is busted, which gets complicated so save this for last - https://stackoverflow.com/questions/63540048/where-exactly-is-macro-vctargetspath-defined
Note:

Changing npm settings such as msvs_version and msbuild_path had no effect (save to break it a different way if you make changes to msbuild_path)
I was able to reproduce both the error in your first and second posts by creating/changing a user environment variable, but it could be stored as a system variable (same spots to check really)
Also this is just my assumptions based on some poking around, take some time to crosscheck this train of thought and consider any replies/corrections from other responders.

Checking for it in the first place....

You can search using Powershell ps>$Env:VCTargetsPath or list everything via CMD cmd>set. There is a GUI option if you prefer and its somewhere in Control Panel
If this variable exists I'd copy/paste the details and delete it to test whether the defaults are working fine, there is a number of ways to do this so best to have a google.
Any time you change an environment variable they wont be picked up by apps immediately. Logging out is usually good enough but given your hassle with this one I'd go scorched earth and just reboot.
After you've logged back in ideally erlpack etc should install without anything additional required

Alternatively if VCTargetsPath doesn't already exist you may be able to use this same  environment variable as a workaround by using it to force node-gyp to use the correct path for MSBuild under 2022.

Caveat on this is that things that need to use MSBuild (not just npm/node) will be bound to this directory. Something to keep in mind if you experience strange issues after making any changes.
To that point I'd recommend creating it as a 'user' not a system variable to start with
Similar to removing an environment variable, I'll leave you to google how to add.
What you'll want to be careful of is the directory you point it to and based on your logs I would expect it to be the following, though important to check
C:\Program Files (x86)\Microsoft Visual Studio\2022\Community\MSBuild\Microsoft\VC\v170\
You may see other folders under that 'VC' path; v170 for Visual Studio 2022, v160 for Visual Studio 2019, or v150 for Visual Studio 2017
Inside that v170 folder you should find 'Microsoft.Cpp.Platform.targets' amongst other files
Save the variable and as with before give the computer an off/on before re-attempting to ensure the variables taken hold.

For a little extra context form when I was testing:

Firstly the error here occurs when you have the VCTargetsPath variable pointed to the location containing 2017's Build tools

C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets(67,5): error MSB8020: The build tools for v143 (Platform Toolset = 'v143') cannot be found. To build using the v143 build tools, please install v143 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj]

And mine

C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets(67,5): error MSB8020: The build tools for v143 (Platform Toolset = 'v143') cannot be found. To build using the v143 build tools, please install v143 build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [S:\Projects\Scrap\node_modules\erlpack\build\vendor\zlib.vcxproj]

Then when you uninstalled 2017 entirely, you received a slightly different error as the folder or its contents no longer exists
I've simulated this by pointing that environment variable to an empty drive

C:\Users\callu\Desktop\Dev\Youtube Music\node_modules\erlpack\build\vendor\zlib.vcxproj(21,3): error MSB4019: The imported project "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Default.props" was not found. Confirm that the expression in the Import declaration "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\VC\VCTargets\\Microsoft.Cpp.Default.props" is correct, and that the file exists on disk.

And again mine...

S:\Projects\Scrap\node_modules\zlib-sync\build\deps\zlib.vcxproj(21,3): error MSB4019: The imported project "S:\Microsoft.Cpp.Default.props" was not found. Confirm that the expression in the Import declaration "S:\\Microsoft.Cpp.Default.props" is correct, and that the file exists on disk.


[requirements] Upgraded responses library to < 0.9

Fixes #53.

Coverage remained the same at 100.0% when pulling f3782e0874b539839a15308489d9196863dacccc on AZtheAsian:master into 8ffed16f76b786c1d66526322069b0f8829c938f on ninuxorg:master.

Coverage remained the same at 100.0% when pulling f3782e0874b539839a15308489d9196863dacccc on AZtheAsian:master into 8ffed16f76b786c1d66526322069b0f8829c938f on ninuxorg:master.


Fix inability to transition between specific presets in Venstar component

Description:
This change addresses a bug where one is unable to change directly between the away and temperature hold presets, as temperature hold cannot be enabled on a Venstar thermostat if away mode is active.
Furthermore, this change removes redundant state checks as the set_away and set_schedule calls are idempotent in the venstarcolortouch library.
See https://github.com/hpeyerl/venstar_colortouch/blob/master/src/venstarcolortouch/venstarcolortouch.py#L275.
Checklist:

[X ] The code change is tested and works locally.
[X ] Local tests pass with tox. Your PR cannot be merged unless tests pass
[X ] There is no commented out code in this PR.
[X ] I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

[NA ] Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

[X] The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
[X] New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
[X] Untested files have been added to .coveragerc.

If the code does not interact with devices:

[NA] Tests have been added to verify that the new code works.

This was next on my list, thanks!


Parsing Problems?

Hello i installed the i.cal Apapter but I get Noting in Events.
I think its an issue with the parse of the basic.ics (maybe google calendar Changed the ics file?).
On Youtube Videos i saw that the Feld HTML normaly include a List with Events and no html Tags and i Think the Folders with many numbers represens every calender entery.
On Debug Log i see, its principial working:
2019-09-24 14:34:22.778  - debug: ical.0 {"9":{"type":"VTIMEZONE","params":[],"tzid":"Europe/Berlin","LIC-LOCATION":"Europe/Berlin","a":{"type":"DAYLIGHT","params":[],"tzoffsetfrom":"+0100","tzoffsetto":"+0200","tzname":"CEST","start":"1970-03-29T01:00:00.000Z","datetype":"date-time","rrule":"RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU"},"da":{"type":"STANDARD","params":[],"tzoffsetfrom":"+0200","tzoffsetto":"+0100","tzname":"CET","start":"1970-10-25T02:00:00.000Z","datetype":"date-time","rrule":"RRULE:FREQ=YEARLY;BYMONTH=10;BYDAY=-1SU"}},"cg@google.com":{"type":"VEVENT","params":[],"start":"2019-09-28T15:00:00.000Z","datetype":"date-time","end":"2019-09-28T16:00:00.000Z","dtstamp":"2019-09-24T12:34:18.000Z","uid":"cg@google.com","created":"2019-09-23T21:53:36.000Z","description":"","lastmodified":"2019-09-23T21:53:36.000Z","location":"","sequence":"0","status":"CONFIRMED","summary":"Nina Geburtstag ","transparency":"OPAQUE"},"cg<EMAIL_ADDRESS>On the Feld HTML i get this:
Noch 0 Stunden Zweirad wellbrock anrufen
Morgen   antwort erhalten?
Morgen 08:00-09:00 Neue brillengläser machen lassen
Morgen  FOLSÄURE NEHMEN
Übermorgen 11:00-12:00 metrotexat rezept und info
Übermorgen 22:00-23:00 Antwort erhalten?
In 4 Tagen 17:00-18:00 Nina Geburtstag
In 5 Tagen 10:00-11:00 Begegnungsstätte Kinder programm
In 6 Tagen 17:30-18:00 Wüstenrot
In 6 Tagen  MTX SPRITZE NEHMEN
on JS iCal table i get This:
[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object],[object Object}
i think that is why i get olny the entery vacation in Every Folder.
Here is a screenshot:
https://s17.directupload.net/images/190924/93zgmdtl.png
Hope This get be Fixed soon.

I have the same problem:
Standard Google Calender url -> only "vacation" entries in events.

Any way i might help to solve this?


fix debug info on single-line-map page

Hi @moshe742!
Shana Tova, and welcome to the open bus repository.

please feel free to slack me (or comment here) if you have any questions or troubles.
Since I see no changes in this PR, I'll change it to a draft.
Please mark it as "ready to review" when you'll be ready.


refer function on webRTC question

These are typically directed to the google group.
Hi,
Currently I'm working with 0.17.1.
when i using logLevel: "debug" and i make outbound out-of-dialog REFER requests. Status code only send me status 100 trying
and not auto hangup in spite of target refer accepted, after accepted 30s code send status code 408 request time out, still not hangup and make noise between caller and refer target and when i reject the call, it remains the same . Until i must hangup middle call manually.
Can someone please give me the solution (a simple code example would be very much appreciated).
Thanks!

How are you sending the refer?

How are you sending the refer?
First i receive call then using session to refer like this:

    const transferTarget = SIP.UserAgent.makeURI(`sip:${toNumber}@${TestSipjs.domain}`);

    window.SimpleUser.session.refer(transferTarget,{
      onNotify(e){
        console.log(e);
      },
      requestDelegate: {
        onAccept(){
          window.SimpleUser.session.bye();
          console.log("ok");
        },
        onReject(){
          self.setupInCall();
        }
      }
    }

);

Creating a transfer (REFER) with SimpleUser is not supported. You need to use the full API.
Out of dialog refer is not supported at all with SIP.js at the moment. Your hack is an in dialog refer.
This is definitely not a SIP.js bug but a question about implementation. Please direct this towards the Google Group


[FLINK-26130][docs] Expanded the reason to increase network buffer si…

…ze in documentation

What is the purpose of the change
Expand https://nightlies.apache.org/flink/flink-docs-release-1.14/docs/deployment/memory/network_mem_tuning/#the-number-of-in-flight-buffers documentation
Brief change log

Clarified the reason to increase the network buffer size

Verifying this change
It is documentation
Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (yes / no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
The serializers: (yes / no / don't know)
The runtime per-record code paths (performance sensitive): (yes / no / don't know)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (yes / no / don't know)
The S3 file system connector: (yes / no / don't know)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

Could you please update the commit with the correct JIRA ticket ;)

@dawidwys, thanks. for some reason I used wrong ticket number in branch and commit names


[FloatingActionButton] Animate appearing of the FAB

[x] PR has tests / docs demo, and is linted.
[x] Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
[x] Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Closes #5241
It's a base version which should be changed to comply next branch. Assumes that PR #5246 is applied.

The animation is really something MUI hasn't invested much. I'm glad people start investing into that field. Thanks.
However, we are migrating component to the next branch and prioritizing bug fixes for the master branch. If you want to contribute you gonna have two options:

Continue the conversation so we can rebase that PR for the next branch. That's quite some work
Create a package with that component so the community can use it.

I'm interested in merge this (depend on how much of work this requires).
Should I look to rebase to "v1-alpha" branch instead of "next"?
What point of the start do you advise?

The package has been published: https://www.npmjs.com/package/material-ui-animated-fab


Migrate Qaterial from Qt5 to Qt6

Should work with all Qt 6 version.

Superseded by https://github.com/OlivierLDff/Qaterial/pull/157.
Feel free to add something in a new PR if something is missing from my pr


Implement event/hook emitter

This PR relates to #177, #174, #150, #144.
After an interesting discussing on Slack, I've modified the emitter implementation. Below is a detailed explanation of what have changed since my last comment on issue #174.
From the Slack discussion it was clear that subscriptions is the best name to group both events and hooks since @jbucaran wants to avoid having both. The following is a list of all events and hooks emitted by the core of Hyperapp:
subscriptions: {
    '*': (type) => {},                   // Listens to all events/hooks
    'actions': (name, data) => {},       // When any action is called
    'action:save': (data) => {},         // When a specific action is called
    'update': (name) => {},              // When model updates
    'load': (model, actions) => {        // When the core loads right before the first render
        if (model.contacts.length < 1) {
            actions.contacts.addRandom()
        }
    },
    'render': (model, view) => {         // When rendering
        // do something and
        // return a modified view
        return view
    }
}

I have integrated Mitt in Hyperapp storing all handlers on the subscription object. Mitt methods on and off have been removed. The only way to register event/hook handlers now is via the subscriptions object when initialising the app/plugin.
Also, Mitt is no longer exposed to the app. Instead only actions will receive an emit object allowing them to trigger an event and/or hook:

emit.event - is a way to notify that something happened without expecting any value to be returned.

actions: {
  router: {
    match: match,
    go: function (_, data, actions, emit) {
      history.pushState({}, "", data)
      actions.router.match(data)
      emit.event('route:update', data)
    }
  }
}

emit.hook - expects a value to be returned allowing the app/plugin to act based on it. Under the hood this method uses Array.reduce to store the value returned since more than one handler may exist for a given hook. The value that will be returned from the hook is stored on the last argument allowing the app/plugin to change or replace this value.

function render(model, view) {
  view = emit.hook("render", model, view)

  element = patch(root, element, node, node = view(model, actions))
}

In this example from the core, any handler that subscribes to the render hook will be able to change or completely replace the view, which will be returned since it's the last argument. The router subscribes to the render hook:
subscriptions: {
  "render": function (model, view) {
    return view[model.router.match]
  }
}

onLoad is now load and must defined inside subscribers. It also no longer receives the emitter as I first proposed.
onRender is now render for short.
These changes maintain the flexibility brought by an event emitter while limiting its use to avoid undesired side effects.
Prefixing core events/hooks and encouraging plugin authors to do the same is a good idea, keep that mind.

Wow, that's a good explanation 👍

Really great explanation indeed.. I will review the actual code as soon as I get a free moment. Much of what you are saying a lot of sense! 🤓

@pedroborges I don't think it's a good idea to pass the emitter to actions. It will confuse users or open the door to interact with the application with something other than actions.

@jbucaran error is already the forth argument on the action. It does basically the same thing, it send a message to an event handler, except it's limited for just errors.
I thought of adding it to actions as a way for plugins to talk to the app. The router examples I showed on #174 explains what the benefits are.
Do you have any other idea on how to solve cases like that?

@pedroborges Plugins can talk to the app by adding new actions, e.g. Router.

@pedroborges Thanks for all the good work man! The emitter made it to the finals and won after all.


Query selection menu with tooltip instead of textwrap

Currently when hovering an element in the selection menu it wraps over multiple lines to see the full content. This is not optimal with long query names because everything jumps while hovering and hops back when hovering. This is a pretty goofy effect...

A solution could be a tooltip.


[BUG] When edit text from "EditStringDialog" and add "new_line", the text gets broken in the table

Steps to reproduce:

Create a new database
Add a new table
Add a new property and set the data type as string
Add a data row
Click the EditString button from the data
Edit text and add a new line (press ENTER key)
Add more text
Save the database
Observe that the JSON file is broken

TODO: translate the new lines from the EditString panel into "\n"

This should be fixed with the commit:
https://github.com/radubolovan/Godot-Database-Manager/commit/01361e391ebc6bc3de13b9581c17c6b31e94c2aa


Update Composer dependencies (2022-12-16-00-12)

Visual regression test passed!
View the visual regression test report