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

Already on GitHub? Sign in to your account

ENH: integrate: add 'initial' kw to cumtrapz, which makes len(out) == le... #125

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Owner

rgommers commented Jan 4, 2012

...n(in).

Default is initial=None, to be backwards compatible
(meaning len(out) == len(in - 1) along axis of integration.

Also add tests and a decent docstring.

Trac ticket is http://projects.scipy.org/scipy/ticket/1574

@rgommers rgommers ENH: integrate: add 'initial' kw to cumtrapz, which makes len(out) ==…
… len(in).

Default is initial=None, to be backwards compatible
(meaning len(out) == len(in - 1) along axis of integration.

Also add tests and a decent docstring.
aa136e4

Using asarray here breaks the test 'if x is None' in the next line. You'll need to test 'x is None' before this.

When initial is not None, this code does not use the value of initial; it always puts 0 in the initial position, regardless of the value of initial:

In [8]: y
Out[8]: [1, 2, 4, 0]

In [9]: x = arange(len(y))

In [10]: cumtrapz(y, x)
Out[10]: array([ 1.5,  4.5,  6.5])

In [11]: cumtrapz(y, x, initial=0)
Out[11]: array([ 0. ,  1.5,  4.5,  6.5])

In [12]: cumtrapz(y, x, initial=1)
Out[12]: array([ 0. ,  1.5,  4.5,  6.5])

I would expect the last result to be [1.0, 2.5, 5.5, 7.5].

(I just fixed the previous line note to include the values of x and y.)

As long as you are adding tests, could you also add a test or two in which x is not given, perhaps one with the default dx and another with a dx given explicitly?

Owner

rgommers commented Jan 5, 2012

Hmm, a bit too sloppy here, just while we're talking about code quality:) Will fix this tonight.

Owner

rgommers commented Jan 6, 2012

All fixed.

Owner

teoliphant commented Jan 10, 2012

What needs to be done for this to be merged? This looks pretty good.

Member

WarrenWeckesser commented Jan 13, 2012

Hmmm... I was thinking initial would be the initial condition for the accumulation, not just the value to put at the beginning. E.g. cumtrapz(y, x, initial=99) is the same as 99 + cumtrapz(y, x, initial=0). But I don't have a particularly compelling use-case for this, and it makes the use of initial slightly harder to explain. Anyone else see any advantage in that alternative?

Owner

teoliphant commented Jan 13, 2012

I can see your point, Warren. However, I'm more persuaded by "initial" as just a mechanism to keep the output and the input the same length. This has more to do with the "initial" region (i.e. the area under the curve between the non-existent previous value of x and x[0]). Thus, it really should be limited to the number to place in the first element of the output rather than essentially corresponding to a bias in the entire curve.

Typo: "than" (not "then")

Member

WarrenWeckesser commented Jan 14, 2012

Sure, that's fine with me.

Ralf, I just noticed a typo in a docstring (noted in a line comment); other than that, this looks ready to go.

Owner

rgommers commented Jan 21, 2012

Okay, I'll fix the typo and merge this in ~2 weeks. If anyone wants to do it before then, please go ahead.

Owner

rgommers commented Jan 31, 2012

Pushed in commit 6eb2da0.

@rgommers rgommers closed this Jan 31, 2012

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