Opened 4 years ago
Closed 4 years ago
#23062 closed enhancement (fixed)
Document disp option of minimize
Reported by:  mforets  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  numerical  Keywords:  documentation, optimization 
Cc:  dimpase, dcoudert  Merged in:  
Authors:  Marcelo Forets  Reviewers:  Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  83e9a04 (Commits, GitHub, GitLab)  Commit:  83e9a04d74485488739204f758baebe4f37989f4 
Dependencies:  Stopgaps: 
Description
The disp
keyword of the toplevel minimize
function does not appear in the docstring. Moreover, it is currently set up to be verbose by default (don't think is a good idea), hence displays a message similar to:
Optimization terminated successfully. Current function value: 0.000000 Iterations: 28 Function evaluations: 35 Gradient evaluations: 35
Change History (18)
comment:1 Changed 4 years ago by
comment:2 Changed 4 years ago by
 Branch set to u/mforets/23062
 Cc dimpase dcoudert added
 Commit set to dec36b7b1e032addaa67c7e0035433b441e6cbc9
i'm seeking your opinion about: verbose mode by default yes or not?
as stated above, i'm +1 for no convergence message by default.
New commits:
dec36b7  add disp input to docstring

comment:3 Changed 4 years ago by
If you want to make disp=False
the default, it should be done via the deprecation route, as it changes the output.
comment:4 followup: ↓ 5 Changed 4 years ago by
I have no opinion for this change since I never use these methods.
Note that I find the keyword verbose
more usual than disp
.
comment:5 in reply to: ↑ 4 Changed 4 years ago by
Replying to dcoudert:
I have no opinion for this change since I never use these methods. Note that I find the keyword
verbose
more usual thandisp
.
name disp
comes from the backend, it is not something we could change, it seems.
comment:6 Changed 4 years ago by
more precisely, verbose
would be nice and easy to have, assuming the backend does not use it for something else. Otherwise it looks to complicated to do this change.
comment:7 Changed 4 years ago by
from that side i think it can be renamed inside minimize
; it is a true/false flag that is passed to SciPy?'s optimize methods.
(fprime
has been renamed to gradient
, for instance.)
inside the numerical module, the verbose
keyword is currently used in:
 interactive simplex method (
inject_variables
) knapsack
problem
OTOH, disp
is used only here, on minimize
.
comment:8 Changed 4 years ago by
OK, let's go with verbose
instead.
comment:9 Changed 4 years ago by
 Commit changed from dec36b7b1e032addaa67c7e0035433b441e6cbc9 to be7fcf5fd3d43025f90c567ddfe3f4ef47c167d6
comment:10 Changed 4 years ago by
 Commit changed from be7fcf5fd3d43025f90c567ddfe3f4ef47c167d6 to 469bd82e99d77842bad557889787eb72abc4fabb
Branch pushed to git repo; I updated commit sha1. New commits:
469bd82  set default > False, and update doctests

comment:11 Changed 4 years ago by
change are done ==> essentially for review..
although i doubted is if it's the good deprecation warning that i chose. from the list in the Guide, there is no explicit mention about "changing the output".
comment:12 followup: ↓ 14 Changed 4 years ago by
here is another thing with this function: suppose you choose algorithm='ncg'
. then, the line with optimize.fmin_ncg
does not get executed if f
is a Python function?! (not even an exception?)
comment:13 Changed 4 years ago by
 Commit changed from 469bd82e99d77842bad557889787eb72abc4fabb to 540bdeac0538207c5a0d1bf3613a91dbdba9e41d
comment:14 in reply to: ↑ 12 Changed 4 years ago by
Replying to mforets:
here is another thing with this function: suppose you choose
algorithm='ncg'
. then, the line withoptimize.fmin_ncg
does not get executed iff
is a Python function?! (not even an exception?)
i had closer look into this, and created #23074
comment:15 Changed 4 years ago by
 Commit changed from 540bdeac0538207c5a0d1bf3613a91dbdba9e41d to 83e9a04d74485488739204f758baebe4f37989f4
Branch pushed to git repo; I updated commit sha1. New commits:
83e9a04  remove pair of backticks (all or nothing)

comment:16 Changed 4 years ago by
 Status changed from new to needs_review
no further changes expected (from my side) ==> needs review
comment:17 Changed 4 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from needs_review to positive_review
Looks good to me, thanks!
comment:18 Changed 4 years ago by
 Branch changed from u/mforets/23062 to 83e9a04d74485488739204f758baebe4f37989f4
 Resolution set to fixed
 Status changed from positive_review to closed
homework:
minimize_constrained