bugGNU Octave - Bugs: bug #36732, interp1 does not check input for...

 
 

bug #36732: interp1 does not check input for monotonicity

Submitter:  Francesco Potortì <pot>
Submitted:  Tue 26 Jun 2012 12:16:20 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Inaccurate Result
Status:  Fixed Assigned to:  bpabbott
Originator Name:  Francesco Potortì Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 24 Aug 2012 12:44:23 AM UTC, comment #61: 

Yes, I think that is a good idea.

Ben Abbott <bpabbott>
Group Member
Thu 23 Aug 2012 01:30:19 PM UTC, comment #60: 

Ben,

The changeset looks good to me.  Would it be useful
to add the the help text a line like:

If the input X is sorted it is used directly.  If not,
the (X,Y) pairs are sorted on X.

And, of course, the Manual needs updating.

Michael Godfrey <godfrey>
Group Member
Thu 23 Aug 2012 12:17:35 PM UTC, comment #59: 

In my last comment, I neglected to encourage additional comments if there are any remaining concerns.

Ben Abbott <bpabbott>
Group Member
Thu 23 Aug 2012 12:10:38 PM UTC, comment #58: 

I've pushed a changeset to make testing more convenient ... unfortunately, I forgot to modify the changelog description :-(

http://hg.savannah.gnu.org/hgweb/octave/rev/94d512d712e3

Ed, I hesitate to change the choice of left/right continuity as this was decided quite some time ago (several years I think).

The example below may be of interest.


X = [ 2 1 3 2];
Y = [ 9 1 3 10];
% These are right-continuos (returns 10 at X=2)
y1 = interp1 (X, Y, 2)
[~, n] = sort (X);
y2 = interp1 (X(n), Y(n), 2)
[~, n] = sort (X, "descend");
y3 = interp1 (X(n), Y(n), 2)
y4 = interp1 (X, Y, 2, "-right")
% These are left-continuous (returns 9 at X=2)
[~, n] = sort (fliplr (X), "descend");
y5 = interp1 (X(n), fliplr(Y)(n), 2)
y6 = interp1 (X, Y, 2, "-left")


Ben Abbott <bpabbott>
Group Member
Wed 22 Aug 2012 07:54:50 PM UTC, comment #57: 

maybe it's because I'm left-handed and/or dyslexic but it seems to me that if x is increasing the interpolant should be left-continuous at a discontinuity, and right-continuous if it is decreasing.

Edward Meyer <eem2314>
Tue 21 Aug 2012 11:57:38 PM UTC, comment #56: 

Regarding Michael's suggestion to use sort() in comment #52 in order to switch form right to left-continous isn't so straight forward.  Consider the example below, where yR will be right-continous by default.


X = [ 2 1 3 2];
Y = [ 2 1 3 10];
x = 1:0.5:3
yR = interp1 (X, Y, x);


This is equivalent to


yR = interp1 ([1 2 2 3], [1 2 10 3], x)


If sort(..., "descend") is applied to X and Y is modified accordingly we get ...


X = [ 2 1 3 2];
Y = [ 2 1 3 10];
[X, n] = sort (X, "descend")


Which results in


X =   3   2   2   1
n =   3   1   4   2
+verbatim

Using "n" to reorder "Y" results in;

+verbatim+
Y = Y(n)

Y =    3    2   10    1


Notice that Y(2) and Y(3) are not what is intended.  Meaning, that the example below still produces a right-continuous result.


X = [ 2 1 3 2];
Y = [ 2 1 3 10];
[~, n] = sort (X, "descend");
x = 1:0.5:3
yR = interp1 (X(n), Y(n), x);


In order the obtain a left-continuous result ...


X = [ 2 1 3 2];
Y = [ 2 1 3 10];
[~, n] = sort (flipud (X), "descend");
x = 1:0.5:3
yL = interp1 (X(n), fliplr(Y)(n), x)
yL =

   3.0000   2.5000   2.0000   5.5000   1.0000

yR = interp1 (X, Y, x)
yR =

    1.0000    1.5000   10.0000    6.5000    3.0000


My use of fliplr() may be misleading, since internally interp1() operates along the rows and supports a 2D Y.  Meaning, the internal approach to doing this is different from this example.

Ben Abbott <bpabbott>
Group Member
Tue 21 Aug 2012 11:16:01 PM UTC, comment #55: 

I've added options to force the interpolation to be right/left-continuous.

Ben,
I do not think that this is a good idea.  It is just as simple
to cause this by sorting the data before the call to interp1.
This only adds another option to cause more confusion.

I would much prefer to leave the behavior of interp1 as it was.

Also, someone claimed that duplicate entires that were previously
allowed have been prevented. This change does not, I think, help.

I think that fixing the documentation so that it matches
the code and so that people can understand what the behavior was
should be done first.

Michael Godfrey <godfrey>
Group Member
Tue 21 Aug 2012 11:08:48 PM UTC, comment #54: 

I've added options to force the interpolation to be right/left-continuous.

Left-continuous is forced by "-left" and right-continuous by "-right".

(file #26413)

Ben Abbott <bpabbott>
Group Member
Tue 21 Aug 2012 01:48:43 PM UTC, comment #53: 

The question of the merits of "discontinuous interpolant"
is a separate matter.  It should not be the subject of this
bug report.

It might be better to start a separate thread to discuss this.

Michael Godfrey <godfrey>
Group Member
Tue 21 Aug 2012 01:26:14 PM UTC, comment #52: 

Finally, I do not see the need for statements like
 X=sort(X,'descend')

is worth this much worry.

It would not be a good idea to introduce incompatible
behavior in interp1 in order to avoid the need for
2 simple lines of code.

Michael Godfrey <godfrey>
Group Member
Tue 21 Aug 2012 12:39:36 PM UTC, comment #51: 

This bug is still being discussed on the
maintainers list.  The discussants seem not to
have read the code. The code:

 ## check whether x is sorted; sort if not.
  if (! issorted (x, "either"))
    [x, p] = sort (x);
    y = y(p,:);
  endif

is ALWAYS exected by intrp1.m. This is why the documentation
should say the data are sorted as shown above. 
It is hard to see why this causes a problem.

Michael Godfrey <godfrey>
Group Member
Tue 21 Aug 2012 01:12:08 AM UTC, comment #50: 

Ben,

I think you are correct.  The only change to the code
that I intended was the checking for more than a "pair" of
equal values.  The other check was a warning about (Matlab)
incompatibility.  Not important.

I had only done part of the documentation updates.  Great
if you have improved that.  The key thing that I had not done
is to make clear that the X,Y data are sorted, so the order
of the elements in the input vectors does not matter at all.

And, very good that you dealt with the discontinuous interpolation.

I do not see any need to change the behavior.  The warning
about Matlab compatibility does not change the behavior.

Michael

Michael Godfrey <godfrey>
Group Member
Tue 21 Aug 2012 01:04:57 AM UTC, comment #49: 

After spending time experimenting (I apologize for not doing so earlier), it appears to me that the original implementation of interp1.m was consistent with its documentation.  I'm attaching a changeset which reverts the recent change and makes some minor changes to the documentation.

I've also added two tests, and a demo, to clarify how the discontinuous interpolation was implemented.

Since the documentation describes that the implementation permits a piece-wise discontinuous interpolant, this doesn't quality as a bug.  If we are to change the behavior, it should be discussed on the maintainers list and a consensus reached before doing so (since it would break existing code).

I'll wait on discussion before I push the changeset.

(file #26404)

Ben Abbott <bpabbott>
Group Member
Mon 20 Aug 2012 02:30:02 PM UTC, comment #48: 

Although i was not referring to interp1.m being not qualified as a function. But with the example you gave:

interp1 ([1 2 2 3], [1 2 3 4], 2)

it shows that it can have two possible values (either 2 or 3) which means interp1 doesn't qualify as a function which is of course incorrect!

I guess this argument shows why X should have distinct values.

With regards,
Vivek

Vivek <vivek91m>
Mon 20 Aug 2012 01:51:24 PM UTC, comment #47: 

Actually i am talking about the relation between input values X and Y being not qualified as function, if X has identical values.

example:
X = [1 2 3 4];
Y = [4 6 8 10];

is a function.

X = [1 2 2 4];
Y = [4 6 8 10];

is not a function.

And if Octave allows X to have identical values then it will create ambiguity as you explained with the example.

Yes you are absolutely correct about the example you gave
"interp1 ([1 2 2 3], [1 2 3 4], 2)"
I was referring to the same point. Octave gives 3 which is ambiguous. It is not desired! the answer can also be 2.

Vivek <vivek91m>
Mon 20 Aug 2012 01:43:05 PM UTC, comment #46: 

I think the current implementation does qualify as a function.  There is only one output for a given input.

I must admit, I'm not sure how what interp1 ([1 2 2 3], [1 2 3 4], 2) should return. Thus, I think the point on ambiguity is apt.  The current sources returns 3.  Is this what is desired?  If so, perhaps that should be made clear in the documentation.



Ben Abbott <bpabbott>
Group Member
Mon 20 Aug 2012 01:40:29 PM UTC, comment #45: 

In any practical situation (in experiments or something) whenever X has some identical values, it should be left over to the user what needs to be done (taking maximum, minimum, average etc) with those values  according to the user's problem. It should not be given to Octave (or any other language) to decide.

(And by allowing one identical pair, octave is not serving any purpose but only creating confusion).


Vivek <vivek91m>
Mon 20 Aug 2012 01:27:09 PM UTC, comment #44: 

Each material i referred to on interpolation have the basic requirement of X having distinct values.

As a matter of fact, if i allow identical values then the relation doesn't remain a function according to the definition of a function,
(In mathematics, a function is a relation between a set of inputs and a set of permissible outputs with "the property that each input is related to exactly one output.")

Vivek <vivek91m>
Mon 20 Aug 2012 01:06:10 PM UTC, comment #43: 

I am really not getting the point of discontinuous interpolant. I went through different books and related material but dint came across any concept related to it.

Theoretically also, it seems incorrect to me (according to the examples i gave earlier). This will create ambiguity, because
for linear piecewise approximation, Interp1 draws segments between successive points. So in case of one identical pair, the question will be which one to choose? It's a decision Octave (or any other language) can't do for you.


It is problem specific whether i want select to use the minimum, the maximum, the median, or the mean of those two identical values etc.

Regarding monotonicity, everything will be either increasing or decreasing after sorting. User should be free to input the values at random.

Vivek <vivek91m>
Mon 20 Aug 2012 01:03:54 PM UTC, comment #42: 

There is no difference between right and left-continuous interpolants correct?

This is just due to whether the sort was assending or decending.
It should all be symmetrical.

Michael Godfrey <godfrey>
Group Member
Mon 20 Aug 2012 12:55:44 PM UTC, comment #41: 

I was referring to the part below.

"The discontinuous interpolant is right-continuous if x is increasing, left-continuous if it is decreasing."

There is no difference between right and left-continuous interpolants correct?

Ben Abbott <bpabbott>
Group Member
Mon 20 Aug 2012 12:38:49 PM UTC, comment #40: 

The only change that I think may be needed
in the Manual would be to explain the sorting.
This should probably go in the help text too.

Do you think of other needed changes?

I will take a look at this soon.

Michael Godfrey <godfrey>
Group Member
Mon 20 Aug 2012 12:23:02 PM UTC, comment #39: 

I had inferred the "increasing" / "decreasing" values referred to the "xi" values (i.e. interp1 (x, y, xi)).

Francesco's thought that they referred to the "x" values prior to the sorting makes more sense.

We'll need another changeset to fix the manual, correct?

The "distinct" language troubles me since Octave has a "unique()" function.  The use of "distinct" infers to me that it means something different than "unique".  Is there a reason why "distinct" is being used as opposed to "unique"?

Ben Abbott <bpabbott>
Group Member
Mon 20 Aug 2012 11:41:32 AM UTC, comment #38: 

Francesco,

Your comments seem right except for the fact that
the sort may be in either increasing or decreasing
order.  This may explain the part about right or left
continuity.

The current patch allows more than one equal pair,
but issues a warning that this is "not compatible."


Michael Godfrey <godfrey>
Group Member
Mon 20 Aug 2012 11:18:19 AM UTC, comment #37: 


>About non-distinct points, I am assuming that the Manual is correct. It says in Section 29.1:
>
>Duplicate points in x specify a discontinuous interpolant. There should be at most 2 consecutive points with the same value.


Most probably, this means that, once the X are sorted, any repeated points should be in couples (i.e. three consecutive repeated points are not allowed), but there is no limit on the number of couples.

>The discontinuous interpolant is rightcontinuous if x is increasing, left-continuous if it is decreasing. Discontinuities are (currently) only allowed for "nearest" and "linear" methods; in all other cases, x must be strictly monotonic.


This makes little sense, because X is sorted in increasing order anyway.
Maybe this was a comment written at a time when there was no sorting in place, which would also explain the reason why the text requires monotonicity.

Francesco Potortì <pot>
Mon 20 Aug 2012 01:46:28 AM UTC, comment #36: 

About non-distinct points, I am assuming that the
Manual is correct. It says in Section 29.1:

Duplicate points in x specify a discontinuous interpolant. There should be at most
2 consecutive points with the same value. The discontinuous interpolant is rightcontinuous
if x is increasing, left-continuous if it is decreasing. Discontinuities are
(currently) only allowed for "nearest" and "linear" methods; in all other cases, x
must be strictly monotonic.

My current patch is attached.

(file #26395)

Michael Godfrey <godfrey>
Group Member
Mon 20 Aug 2012 12:10:48 AM UTC, comment #35: 

Yes. Please attach it.

Also, I'm not sure I understand by what you mean by "non-distinct values".

My thought was that there may be no more than two repeated x-values.  Are you saying that for any x-value there may not be more than two distinct y-values?

Meaning that x = [1 2 2 2 2 3] and y = [1 2 3 4 5 6] is invalid, but x = [1 2 2 2 2 3] and y = [1 2 2 3 3 6] in ok?

I was thinking both of those would produce an error, but x = [1 2 2 3] and y = [1 2 3 6] would be ok.

Ben Abbott <bpabbott>
Group Member
Sun 19 Aug 2012 11:53:13 PM UTC, comment #34: 

Agreed about after sorting. 
I would only suggest a somewhat clearer error:

  error ("interp1: X must have at most one pair of non-distinct values");
  
 
But, now that I look at it more carefully, it does not look
like your patch does this.  Or, am I wrong?

Anyhow, my version gives a warning for non-distinct pairs,
and errors for more non-distincts than 2.

Do you want to see it?

Michael

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 10:09:54 PM UTC, comment #33: 

It goes immediately after sorting X.


diff --git a/scripts/general/interp1.m b/scripts/general/interp1.m
--- a/scripts/general/interp1.m
+++ b/scripts/general/interp1.m
@@ -169,7 +169,7 @@
   endif

  ## check whether sample point @var{x} are distinct; give error if not.
-  if (any (diff (x) == 0))
+  if (any ((diff (x(1:(end-1))) == 0) & (diff (x(2:end)) == 0)))
     error ("interp1: X should have distinct values");
   endif


Ben Abbott <bpabbott>
Group Member
Sun 19 Aug 2012 09:57:50 PM UTC, comment #32: 

Ben,

About the demo:  It looks plausible, but
I have only glanced at it.  Matlab cannot do it, of course.

I just did the test for noncontiguous values, but
your code looks more compact, so I will try it.

But, what is the answer to where the code should go:
before or after sorting X?  Technically, it seems to
me that it should go after.  But, that can (did) confure
people.

Michael

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 09:39:44 PM UTC, comment #31: 

Michael,

I had omitted an important part of the demo


 clf
 xi = [1, 2, 3, 3, 4, 5];
 yi = [1, 2, 2, 4, 4, 5];
 x = 1:0.01:5;
 plot (x, interp1 (xi, yi, x),
       fliplr (x), interp1 (xi, yi, fliplr (x)))


Should this example produce a single curve, or should the ascending curve produce a different result as compared to the descending curve?

From comment #24, I had thought you intended that ascending values of x would use interp1 ([1, 2, 3, 4, 5], [1, 2, 2, 4, 5], x) and descending values of x would use interp1 ([1, 2, 3, 4, 5], [1, 2, 4, 4, 5], x).

I assume my inference was incorrect.

Regarding the sorting, Matlab does this as well.


interp1 ([1, 3, 2, 5, 4], [1, 3, 2, 5, 4], 1:5)

ans =

     1     2     3     4     5


The language may need rewording, but does the code below do what is needed to finish your changeset?


 ## check for non-continuous identical values.
  if (any ((diff (x(1:(end-1))) == 0) & (diff (x(2:end)) == 0)))
    error ("interp1: X may not have non-continuous identical values");
  endif


Ben Abbott <bpabbott>
Group Member
Sun 19 Aug 2012 09:16:36 PM UTC, comment #30: 

I noticed an added feature of interp1 which appears
not to be documented:  The X vector is sorted before
the interpolation is done.  This surely added to
the confusion following the original bug report.

For now, I have put the tests for distinct and
duplicated values ahead of the sort.  But, it may
make sense, once users realize that X is sorted, to
put the test after the sort.

Anyhow, the tests now seem to work.

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 06:21:08 PM UTC, comment #29: 

Ben,

What I am working on now is the test for
non-contiguous, identical values in X.

This was the original bug report. Otherwise,
the original code seems correct.  My last
patchs just correct the documentation.

Your test for any duplicate values is
overkill unless Matlab compatibility is
required.  Your test could be retained as
a warning of Matlab incompatiblity.

OK?

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 05:57:45 PM UTC, comment #28: 

Michael, are you working on a changeset, or should interp1 already handle the right-continuous / left-continuous behavior correctly with Vivek's changeset reverted?

I tried the example below, but get the same curve twice.


x = [1, 2, 3, 3, 4, 5];
y = [1, 2, 2, 4, 4, 5];
a = 1:5;
plot (a, interp1 (x, y, a))


Ben Abbott <bpabbott>
Group Member
Sun 19 Aug 2012 05:18:28 PM UTC, comment #27: 

An example of a X pair is:
interp1 ([1,2,2,3,4],[0,1,4,2,1],[-1,1.5,2,2.5,3.5], "linear", "extrap")

In X = [1,2,2,3,4] 2,2 is a pair.

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 05:00:42 PM UTC, comment #26: 

Could you please give an example of "duplicate pair" as mentioned in the manual, if possible?
and what does discontinuous interpolant mean?

Vivek <vivek91m>
Sun 19 Aug 2012 04:49:43 PM UTC, comment #25: 

Additional patch to interp1.m is
attached.  To be applied after the
previous one, or merged.
This fixes doc as mentioned below.


(file #26393)

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 04:40:09 PM UTC, comment #24: 

Yes, "duplicate pair" is described in
Section 29.1 of the Manual:
Duplicate points in x specify a discontinuous interpolant. There should be at most
2 consecutive points with the same value. The discontinuous interpolant is rightcontinuous
if x is increasing, left-continuous if it is decreasing. Discontinuities are
(currently) only allowed for "nearest" and "linear" methods; in

There is a typo at the end of this paragraph.  I will fix that
soon.  "sitrctly monotonic" should read "distinct"

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 04:33:52 PM UTC, comment #23: 

By duplicate pair, do you mean identical values in X?

Vivek <vivek91m>
Sun 19 Aug 2012 04:31:19 PM UTC, comment #22: 

Minor point: all the corrections are in interp1.m.
interp1.txi appears to be correct.

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 04:26:41 PM UTC, comment #21: 

I read the code and documentation more carefully.
It appears the a pair of duplicate points in X are
allowed.  The documentation is misleading in that
it also says that the X points must be monotonic
(not strictly monotonic).  This is not correct.

So, the new attached interp1_new.diff removes the
test for NO distinct points and corrects the interp1.txi
file.  There should be a test for only one duplicate
pair, so I inserted a FIXME: to that effect. This test
is to be written.

Does everyone agree that the (not Matlab compatible)
duplicate pair should be supported?

I am testing this now.  Comments?

(file #26392)

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 03:33:56 PM UTC, comment #20: 

The update for test for distinct values
of X appears to work, but the incorrect
tests were not changed to test for error.
Patch that does this is attached.


(file #26390)

Michael Godfrey <godfrey>
Group Member
Sun 19 Aug 2012 01:22:55 AM UTC, comment #19: 

Matlab does not require strictly monotonic X.
It does require distinct X.

Michael Godfrey <godfrey>
Group Member
Sat 18 Aug 2012 11:39:22 PM UTC, comment #18: 


In matlab, the following does indeed return an error about values of X not being unique.

interp1([1:5 4:9], 1:11, 4)


and the following works fine and returns the value 4:

interp1([1:5 3.5:9], 1:11, 4)


Carnë Draug <carandraug>
Group Member
Sat 18 Aug 2012 11:36:52 PM UTC, comment #17: 

The following was a comment by Francesco Potortì by e-mail:

As far as I know, Matlab requires strictly monotonic values for X.  The bug report claims that an error is generated in Matlab for

interp1([1:5 4:9], 1:11, 4)


Please, could someone with access to Matlab check whether this one generates an error too?

interp1([1:5 3.5:9], 1:11, 4)

In this second test case X contains all distinct values, but they are non-monotonic.

Carnë Draug <carandraug>
Group Member
Fri 17 Aug 2012 06:18:52 PM UTC, comment #16: 

Right!  Sorry about the confusion.  Distinct is
correct.  And, the Manual is wrong.  So, the
error test should be for distinct values of X
and the manual should be corrected.


Michael Godfrey <godfrey>
Group Member
Fri 17 Aug 2012 06:09:19 PM UTC, comment #15: 

I guess, there is no need for strictly monotonicity.
What should be strictly monotonic? X or Y?

See, putting a restriction over X being strictly monotonic doesn't make sense, it is the relation between X and Y that matters. The relation can be many to one or one to one but not many to one. i.e. only restriction should be that values of X have to be distinct.

Because I can always rearrange the order of X-values and correspondingly Y-values will change.

There is no need for function to be strictly monotonic (sin(X)?)


I guess we should relax strictly monotonicity, only restriction should be that X should have distinct values.

Vivek <vivek91m>
Fri 17 Aug 2012 05:09:48 PM UTC, comment #14: 

Vivek,

Your explanation shows why strict monotonicity
is required.  The function should generate an
error.  Matlab does:

>> interp1 ([1,2,2,3,4],[0,1,4,2,1],[-1,1.5,2,2.5,3.5], 'linear', 'extrap'), [-2,0.5,4,3,1.5] 

??? Error using ==> interp1 at 261
The values of X should be distinct.

But, I think that it would be better to say "strictly monotonic"
instead of "distinct."

Michael Godfrey <godfrey>
Group Member
Fri 17 Aug 2012 04:35:20 PM UTC, comment #13: 

In my opinion repeated values should not be allowed for interpolation, let me explain it with an example: Let's say i have following data points:

 x  f(x)

  1.    4

1    6
2    8
3    10

Now if i want to get f(1.5) then i will take the value midway between f(1) and f(2) which would be 7. Now in case i have repeated values the data points will be:

x   f(x)

  1.    4

1    6
1    7 
2    8
3   10

Now in this case what f(1.5) will be? 7 or 7.5? (What will you call the best interpolation?) There will be ambiguity in this case. Also in the present case the sample points are nicely behaving so there is not much difference between 7 and 7.5. Although in some cases (if let's say f(1) = 6 and f(1) = 9) there can be lot of difference between answers when we choose one data point over another.
So I guess, either octave should not allow repeated x values or it should generate some warning (I don't know what the warning would be?).

With regards,
Vivek.

Vivek <vivek91m>
Fri 17 Aug 2012 04:29:24 PM UTC, comment #12: 

I thought the rule is strictly monotonic
as the documentation says.  In this case
equal values are not allowed.  Not a warning.
An errror should result if 2 values are equal.

Michael Godfrey <godfrey>
Group Member
Fri 17 Aug 2012 03:35:15 PM UTC, comment #11: 

I don't have an objection to repeated x-values.

When there are two identical x-values, I assume the first corresponds to values below and the second to values above?

Shall a warning be given in such cases?

What is to be done when there are more than two identical values?


Ben Abbott <bpabbott>
Group Member
Fri 17 Aug 2012 03:07:33 PM UTC, comment #10: 

I'm not sure that those tests were right to begin with. Matlab would fail on that test since it doesn't accept the first argument to have repeated elements (I actually stumbled upon this just last week when checking fir2() on OF).

I'm not sure if Octave allowing it is a bug or a feature.

Carnë Draug <carandraug>
Group Member
Fri 17 Aug 2012 02:50:41 PM UTC, comment #9: 

After applying this patch, I get:

>>>>> processing /media/GoFlex08/qss/d/src/octave/hg/octave/scripts/general/interp1.m

  *** assert (interp1 ([1,2,2,3,4],[0,1,4,2,1],[-1,1.5,2,2.5,3.5], "linear", "extrap"), [-2,0.5,4,3,1.5])
!!!!! test failed
interp1: X should have distinct values
shared variables   scalar structure containing the fields:
    xp =
        0    2    4    6    8   10
    yp =
       0.00000   0.58779  -0.95106   0.95106  -0.58779  -0.00000
    xi =
       -1.00000    0.00000    2.20000    4.00000    6.60000   10.00000   11.00000
    style = *spline
  *** assert (interp1 ([4,4,3,2,0],[0,1,4,2,1],[1.5,4,4.5], "linear"), [1.75,1,NA])
!!!!! test failed
interp1: X should have distinct values
shared variables   scalar structure containing the fields:
    xp =
        0    2    4    6    8   10
    yp =
       0.00000   0.58779  -0.95106   0.95106  -0.58779  -0.00000
    xi =
       -1.00000    0.00000    2.20000    4.00000    6.60000   10.00000   11.00000
    style = *spline
========================================

devel system on Fedora 17 x86_64 and JIT and GUI enabled.  No errors
before this patch,

Michael Godfrey <godfrey>
Group Member
Fri 17 Aug 2012 12:24:50 PM UTC, comment #8: 

I made another minor change and pushed the changeset.

http://hg.savannah.gnu.org/hgweb/octave/rev/2d5e4d283688


Ben Abbott <bpabbott>
Group Member
Thu 16 Aug 2012 12:15:27 PM UTC, comment #7: 

Vivek

I'm currently not able to build Octave (hope to be doing so in a day or two, when I have time to work on that), so I'm not able to push your changeset yet (perhaps someone else will?).

In any event, I hope you'll continue to contribute to Octave.  Related to that, I see a few small violations of our coding standards.  There should be a space between function names and their parentheses. For example, "diff(x)" should be "diff (x)".

Other than that, the changeset looks good to me.

Ben Abbott <bpabbott>
Group Member
Wed 15 Aug 2012 09:05:43 PM UTC, comment #6: 

Hi Vivek

considering the patch, you should leave the generated changeset on the simple text format. No need to make it a docx file (probably more correct to say that you should not).

Carnë

Carnë Draug <carandraug>
Group Member
Wed 08 Aug 2012 07:07:10 AM UTC, comment #5: 

I am sorry, i was working with old version.
Please ignore the last message.

Vivek <vivek91m>
Wed 08 Aug 2012 06:42:50 AM UTC, comment #4: 

In the documentation it is written that

"The sample points @var{x} must be strictly monotonic"

I guess there is no need for 'x' to be strictly monotonic. Only requirement should be that all the values of x should be distinct.

Also, there is another bug in 'interp1'. Following command should give an error:

interp1([1:4],[1:11],4)


although it gives 4. It is not checked whether the length of first and second argument is same or not.


Vivek <vivek91m>
Thu 26 Jul 2012 04:32:10 AM UTC, comment #3: 

Okay i have made some changes to the interp1 code to check for strictly monotonicity (which it was not doing earlier, although it is written in the documentation). I will submit my patch as soon as i know how to submit it :)

Regards,
Vivek

Vivek <vivek91m>
Wed 25 Jul 2012 06:55:39 AM UTC, comment #2: 

I just wanted ask how can it be assigned to me so as to avoid duplicate efforts.

Vivek <vivek91m>
Tue 24 Jul 2012 02:07:53 PM UTC, comment #1: 

I have tested the same command in MATLAB and it gives some error which should be there.
As this has not been fixed yet. So I have started working on the same and will submit the patch as soon as possible.

Regards,
Vivek

Vivek <vivek91m>
Tue 26 Jun 2012 12:16:20 PM UTC, original submission:  

This one should give an error:

interp1([1:5 4:9], 1:11, 4)

In fact, it gives out 4.  The first argument is not checked.

Also, the documentation should say "strictly monotonic" where it currently says "monotonic".

Francesco Potortì <pot>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #26413:  changeset.patch added by bpabbott (5KiB - application/octet-stream)
file #26404:  changeset.patch added by bpabbott (4KiB - application/octet-stream)
file #26395:  interp1.diff added by godfrey (4KiB - text/x-patch)
file #26393:  interp1_new2.diff added by godfrey (829B - text/x-patch)
file #26392:  interp1_new.diff added by godfrey (1KiB - text/x-patch)
file #26390:  interp1.diff added by godfrey (875B - text/x-patch - corrects tests)
file #26366:  bug36732.patch added by vivek91m (736B - text/x-patch)
file #26363:  bug36732.patch.docx added by vivek91m (13KiB - application/vnd.openxmlformats-officedocument.wordprocessingml.document)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by eem2314 (Posted a comment)
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by carandraug (Posted a comment)
  • -email is unavailable- added by vivek91m (Updated the item)
  • -email is unavailable- added by pot (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-05-16 bpabbott StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2012-08-21 bpabbott Attached File- Added changeset.patch, #26413
    2012-08-21 bpabbott Attached File- Added changeset.patch, #26404
    2012-08-20 godfrey Attached File- Added interp1.diff, #26395
    2012-08-19 godfrey Attached File- Added interp1_new2.diff, #26393
    2012-08-19 godfrey Attached File- Added interp1_new.diff, #26392
    2012-08-19 godfrey Attached File- Added interp1.diff, #26390
    2012-08-17 bpabbott StatusNone Ready For Test
        Assigned toNone bpabbott
        Release3.6.2 dev
        Operating SystemGNU/Linux Any
    2012-08-16 vivek91m Attached File- Added bug36732.patch, #26366
    2012-08-15 vivek91m Attached File- Added bug36732.patch.docx, #26363
    2012-08-15 vivek91m Carbon-CopyRemoved 88748 -

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code