Initial tests for optimize.fsolve()
Added a test problem and two initial tests that just check that nothing croaks: a run without a jacobian and a run with a jacobian. Now, I kind of lost track of the whole workflow discussion, so please tell me what process to follow to get this in. Also, is the huge docstring in TestFSolve.pressure_network() an overkill? Yours, Yosef.
Sun, 22 Mar 2009 21:26:42 +0200, Yosef Meller wrote:
Added a test problem and two initial tests that just check that nothing croaks: a run without a jacobian and a run with a jacobian.
Now, I kind of lost track of the whole workflow discussion, so please tell me what process to follow to get this in.
In general: open a ticket, attach the patch (or, even better, post it to the codereview site and paste the URL to the ticket, or branch your own Git clone and paste an URL to it), finally marking the ticket as needs review. And maybe, ping this mailing list, too. Or, you can just send the patch to this mailing list, but then it's quite possible that it gets lost in the traffic and people forget about it. This one was a trivial change, so I just committed it directly (r5633).
Also, is the huge docstring in TestFSolve.pressure_network() an overkill?
The docstring is ok, not too big, though maybe not indispensable. -- Pauli Virtanen
Hi Yosef 2009/3/22 Yosef Meller <mellerf@netvision.net.il>:
Added a test problem and two initial tests that just check that nothing croaks: a run without a jacobian and a run with a jacobian.
Thanks for your contribution!
Now, I kind of lost track of the whole workflow discussion, so please tell me what process to follow to get this in.
Attach your patch to a ticket, and mark the ticket as "Ready for Review".
Also, is the huge docstring in TestFSolve.pressure_network() an overkill?
That's an interesting test case! The docstring is informative, so I don't think we need to remove it. Regards Stéfan P.S. If you are interested, here are some minor nitpicks about formatting. I don't include it in the main message, because it won't make the difference between a positive and negative review: The paragraph with the formulas can be marked up with two colons: + the pressures and flows in a system of n parallel pipes:: + + f_i = P_i - P_0, for i = 1..n + f_0 = sum(Q_i) - Qtot Remember the space after the paramter name: + flow_rates: float -> flow_rate : float Sentences are capitalised with full stops: + A 1D array of n flow rates [kg/s]. According to PEP08, spaces should be inserted between operators (although you'll see this "rule" being broken all over SciPy): + P = k*flow_rates**2 -> k * flow_rates**2 I guess that could also be k * flow_rates ** 2, but that doesn't feel quite right. Remove the extraneous whitespace at the end and beginning of certain lines. + jac[:n-1,:n-1] = pdiff + jac[:n-1,n-1] = 0 + jac[n-1,:] = np.ones(n) Do not align equal marks (according to PEP08).
ציטוט Stéfan van der Walt:
Now, I kind of lost track of the whole workflow discussion, so please tell me what process to follow to get this in.
Attach your patch to a ticket, and mark the ticket as "Ready for Review".
Like this? http://projects.scipy.org/scipy/ticket/897
P.S. If you are interested, here are some minor nitpicks about formatting. I don't include it in the main message, because it won't make the difference between a positive and negative review: [snip] Do not align equal marks (according to PEP08).
Thanks, I applied what wasn't done for me. As for style, I think aligning equal marks sometimes makes it visually easier to read code (like a table), but this is too small to argue about :)
2009/3/22 Yosef Meller <mellerf@netvision.net.il>:
Like this? http://projects.scipy.org/scipy/ticket/897
Thanks, your tests are included now! See revisions 5633, 5634, 5635. Cheers Stéfan
participants (3)
-
Pauli Virtanen
-
Stéfan van der Walt
-
Yosef Meller