Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

SF#368 PDL::Slatec::polyfit ignores incorrect length of weight piddle; passes garbage to slatec polfit #48

Closed
perldl-bot opened this issue Mar 2, 2015 · 3 comments

Comments

@perldl-bot
Copy link

From http://sourceforge.net/p/pdl/bugs/368

@zmughal
Copy link
Member

zmughal commented Mar 4, 2015

Possible fix:

$w .= $y->zeros + $w;

I'll put together a test.

zmughal added a commit that referenced this issue Mar 5, 2015
This reproduces the case where sending a weight vector ($w) that does
not match the length of the data to fit ($x, $y) causes garbage to be
sent to the underlying Slatec polfit() function.

See <https://sourceforge.net/p/pdl/bugs/368/>, <#48>
for more information.
@zmughal
Copy link
Member

zmughal commented Mar 5, 2015

I created a test out of this bug in the branch slatec-polyfit-weight-len #59.

I can reproduce the problem that this non-deterministically fails under Travis-CI job #68 https://travis-ci.org/PDLPorters/pdl/builds/53132579. I have marked the pdl(1) and Perl scalar cases as TODO tests and they sometimes pass.

I have a possible fix which is currently being tested on the same branch.

zmughal added a commit that referenced this issue Mar 5, 2015
This reproduces the case where sending a weight vector ($w) that does
not match the length of the data to fit ($x, $y) causes garbage to be
sent to the underlying Slatec polfit() function.

See <https://sourceforge.net/p/pdl/bugs/368/>, <#48>
for more information.
zmughal added a commit that referenced this issue Mar 5, 2015
@zmughal zmughal closed this as completed Mar 5, 2015
@zmughal zmughal added sf:fixed and removed sf:open labels Mar 21, 2015
@mohawk2
Copy link
Member

mohawk2 commented Feb 24, 2024

A change I'm just making, to throw an error when broadcasting higher dims over a fewer-dim output, has caught the test made for this; line 23 of the polfit test file exposed that the first param ($x_in) was being used as the template to inflate up $w_in and $a1, but the second param was being passed in with more dims, which was causing $a1 to be overwritten repeatedly.

The reason specifically that @djerius was having a problem originally is that until approximately today, a length-1 dim was getting quietly "inflated" (as I'm calling it, when a size-1 dim gets expanded), even on a [phys] input. That works great for a PP function where the dimincs will just be set to 0 for that. It works less well when you pass a size-1 buffer to an external function but tell it the buffer is much longer. That's also getting caught by the change and throwing an error.

That change has also broken PDL::FFTW3 and PDL::CCS which were supplying insufficiently-sized blocks of zeroes. Use RedoDimsCode and nulls, folks! EDIT PDL::FFTW3 0.18 was in fact actually broadcasting over one test output, but 0.19 doesn't now. EDIT 2 The PDL::CCS problem exposed that the 2.082_01 ArgOrder change to Ops broke CCS with its explicit use of output args and explicit swap parameter. That's now been reverted in 160a15d, and the behaviour captured in a test so it can't happen again, in that and d8cf0ef. Backwards compatibility <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants