[SciPy-Dev] New contribution: bode() function for LTI systems

Bjørn Forsman bjorn.forsman at gmail.com
Sun Jun 3 13:07:02 EDT 2012


On 3 June 2012 01:12, Pauli Virtanen <pav at iki.fi> wrote:
> 02.06.2012 20:16, Bjørn Forsman kirjoitti:
> [clip]
>> https://github.com/scipy/scipy/pull/224 - ENH: ltisys: new bode() function
>> https://github.com/scipy/scipy/pull/225 - ENH: ltisys: make lti zpk
>> initialization work with plain lists
>
> #225 is obvious, and should go in.

I see it has been merged. Thanks.

> #224 -- looks mostly good. However, you don't include a test case, and
> as I'm not familiar with the problem domain, I cannot easily
> double-check that the code works (and therefore, cannot merge it).
>
> Could you include a (simple but nontrivial) calculation where you know
> the correct result, and that uses this code?  You most likely have
> already done a couple of checks like this already, so coding up one or a
> few of them as test cases hopefully is not too much work.

I have checked this bode() against the bode function in Matlab.

I have now added (and pushed) 4 tests; test 1 and 2 sanity-checks the
bode() magnitude and phase data against manually entered data. Test 3
and 4 calculate the magnitude and phase using the same algorithm as
the bode() function, but without actually *using* bode(). This way
you/we will be able to catch regression bugs.

Let me know if there is anything more I need to do to get this merged.

BTW, I think #224 will cause a small merge conflict in test_ltisys.py.
How do you prefer to handle this? I normally rebase but some people
don't like rebasing (so I thought I'd ask before doing it) and I don't
know what github does to a pull-request if I rebase the branch.

Best regards,
Bjørn Forsman



More information about the SciPy-Dev mailing list