[SciPy-Dev] New contribution: bode() function for LTI systems
Sun Jun 3 12:07:02 CDT 2012
On 3 June 2012 01:12, Pauli Virtanen <firstname.lastname@example.org> wrote:
> 02.06.2012 20:16, Bjørn Forsman kirjoitti:
>> 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.
More information about the SciPy-Dev