Yes, I think that is a good idea.

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.

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

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.

maybe it's because I'm lefthanded and/or dyslexic but it seems to me that if x is increasing the interpolant should be leftcontinuous at a discontinuity, and rightcontinuous if it is decreasing.

Regarding Michael's suggestion to use sort() in comment #52 in order to switch form right to leftcontinous isn't so straight forward. Consider the example below, where yR will be rightcontinous by default.
This is equivalent to
If sort(..., "descend") is applied to X and Y is modified accordingly we get ...
Which results in

I've added options to force the interpolation to be right/leftcontinuous.
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.

I've added options to force the interpolation to be right/leftcontinuous.
Leftcontinuous is forced by "left" and rightcontinuous by "right".
(file #26413)

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.

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.

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.

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

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 piecewise 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)

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

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.

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.

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).

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.")

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.

There is no difference between right and leftcontinuous interpolants correct?
This is just due to whether the sort was assending or decending.
It should all be symmetrical.

I was referring to the part below.
"The discontinuous interpolant is rightcontinuous if x is increasing, leftcontinuous if it is decreasing."
There is no difference between right and leftcontinuous interpolants correct?

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.

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"?

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."

>About nondistinct 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, leftcontinuous 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.

About nondistinct 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, leftcontinuous 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)

Yes. Please attach it.
Also, I'm not sure I understand by what you mean by "nondistinct values".
My thought was that there may be no more than two repeated xvalues. Are you saying that for any xvalue there may not be more than two distinct yvalues?
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.

Agreed about after sorting.
I would only suggest a somewhat clearer error:
error ("interp1: X must have at most one pair of nondistinct 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 nondistinct pairs,
and errors for more nondistincts than 2.
Do you want to see it?
Michael

It goes immediately after sorting X.

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,
I had omitted an important part of the demo
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.
The language may need rewording, but does the code below do what is needed to finish your changeset?

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.

Ben,
What I am working on now is the test for
noncontiguous, 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, are you working on a changeset, or should interp1 already handle the rightcontinuous / leftcontinuous behavior correctly with Vivek's changeset reverted?
I tried the example below, but get the same curve twice.

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.

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

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

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, leftcontinuous 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"

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

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

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)

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)

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

In matlab, the following does indeed return an error about values of X not being unique.
and the following works fine and returns the value 4:

The following was a comment by Francesco Potortì by email:
As far as I know, Matlab requires strictly monotonic values for X. The bug report claims that an error is generated in Matlab for
Please, could someone with access to Matlab check whether this one generates an error too?
In this second test case X contains all distinct values, but they are nonmonotonic.

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.

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 Xvalues and correspondingly Yvalues 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,
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."

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)
 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)
 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.

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.

I don't have an objection to repeated xvalues.
When there are two identical xvalues, 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?

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.

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,

I made another minor change and pushed the changeset.
http://hg.savannah.gnu.org/hgweb/octave/rev/2d5e4d283688

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.

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ë

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

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.

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

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

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

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".
