Enhancement Proposals for Typing and Alignment

[janosg]

Two new enhancement proposals

This PR adds two new enhancement proposals. The two originated as one proposal and were only split after the joint proposal was already accepted.

EEP-02 Static typing

This enhancement proposal explains the adoption of static typing in estimagic. The goal
is to reap a number of benefits:

• Users will benefit from IDE tools such as easier discoverability of options and
  autocompletion.
• Developers and users will find code easier to read due to type hints.
• The codebase will become more robust due to static type checking and use of stricter
  types in internal functions.

Achieving these goals requires more than adding type hints. estimagic is currently
mostly stringly typed. For example, optimization
algorithms are selected via strings. Another example are
constraints,
which are dictionaries with a fixed set of required keys.

This enhancement proposal outlines how we can accommodate the changes needed to reap the
benefits of static typing without breaking users' code in too many places.

EEP-03 Alignment with scipy

Since the typing proposal will already introduce some breaking changes and deprecations, we can use the same deprecation cycle to better align some names in estimagic with SciPy and other optimization libraries. This will
make it even easier for scipy users to switch to estimagic. The goals are:

• If we can make code written for SciPy run with estimagic, we should do so
• If we cannot make it run, the user should get a helpful error message that explains
  how the code needs to be adjusted.

We will achieve this by renaming arguments that are compatible with what SciPy' minimize expects to the SciPy names and adding aliases for all other arguments of scipy.optimize.minimize. Some of the aliases are purely meant to give better error messages for users coming from scipy, others add actual functionality.

[codecov]

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/estimagic/batch_evaluators.py	0.00%	2 Missing ⚠️
src/estimagic/visualization/estimation_table.py	0.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
...c/estimagic/benchmarking/get_benchmark_problems.py	93.05% <100.00%> (ø)
src/estimagic/optimization/cyipopt_optimizers.py	21.81% <ø> (ø)
src/estimagic/optimization/pygmo_optimizers.py	92.49% <ø> (ø)
tests/estimation/test_estimate_msm.py	95.69% <ø> (ø)
tests/optimization/test_ipopt_options.py	73.33% <ø> (ø)
tests/visualization/test_profile_plot.py	100.00% <ø> (ø)
src/estimagic/visualization/estimation_table.py	84.36% <0.00%> (ø)
src/estimagic/batch_evaluators.py	59.09% <0.00%> (ø)

[timmens]

I think this will be a big step forward for estimagic and I am excited to see the new interfaces in action. Very nice proposal! 🎉

[ChristianZimpelmann]

First comments after having read about 1/3

[ChristianZimpelmann]

Sounds like an important step forward!

I would actually prefer some of the old names over the new ones (derived from scipy / NlOpt) -- they are much more straight forward (e.g., if estimagic keeps params instead of x0, the meaning of xtol_abs isn't easy to understand. But I also see the advantage of aligning the labels closer to those packages and I am sure you have thought a lot about it.

[hmgaudecker]

I would actually prefer some of the old names over the new ones (derived from scipy / NlOpt) -- they are much more straight forward (e.g., if estimagic keeps params instead of x0, the meaning of xtol_abs isn't easy to understand. But I also see the advantage of aligning the labels closer to those packages and I am sure you have thought a lot about it.

Agreed. In the end, we are talking about a trade-off re potential users here:

1. Clarity, drawing in new users
2. Similarity with existing packages, drawing in switchers

So far, we mostly targeted the first group. I am sure the second group is larger when it comes to scipy, so aligning there might be useful. I am less convinced when it comes to NLOpt.

As a middle ground, can't we keep the old names and allow the other ones as aliases?

[amageh]

Great work, thanks to everyone who added to this! 👏

Some of my thoughts:

• I am not a big fan of some of the renaming changes (especially criterion --> fun) but I agree that alignment with other packages is advantageous and therefore I think it makes sense to do it.
• The proposed idea for algorithms sounds very cool, looking forward to seeing this implemented! I also really like the proposed changes for bounds and constraints.

Congrats on this comprehensive (and well-written) EEP!

[mpetrosian]

Very cool proposed changes and well written proposal!

Fascinated by the pre-commit hook solution for building Algorithm classes!
Just minor points:

• Can you add examples for the usages of the jax-like decorators for numerical differentiation?
• In the renaming table, instead of stopping_maxfun, you probably meant to write stopping_maxfev.
• There is an inconsistency between the naming conventions used in the examples and the proposed names in the renaming table, but I wouldn't rewrite the examples for this.
  Looking forward to working on and using the new estimagic!

[janosg]

Thanks for all of your comments. It is very nice to see that we have such an active community and all of the comments really helped to make this proposal better!

So far the only topic that needs more discussion is the renaming. As @hmgaudecker pointed out, this is a trade-off between absolute clarity on the one side and alignment with the ecosystem and brevity on the other side.

I am for the renaming but don't have a super strong opinion here.

I personally think that fun is not a very good name but I do prefer jac over derivative.

I also find that the NlOpt inspired options are actually more readable than our very long name because ours are so long that they are just overwhelming the eye (e.g. here)

I am against having aliases. In the long run this is just too much maintenance and potential for confusion. We would do it in the deprecation phase.

I am opening two polls in Zulip so we can have a vote about this.

[hmgaudecker]

I also find that the NlOpt inspired options are actually more readable than our very long name because ours are so long that they are just overwhelming the eye (e.g. here)

To bring something else to the table, what about a custom convergence object?

[janosg]

I also find that the NlOpt inspired options are actually more readable than our very long name because ours are so long that they are just overwhelming the eye (e.g. here)

To bring something else to the table, what about a custom convergence object?

Custom convergence objects would be nice. A super nice example of custom convergence objects that can be combined with boolean operations is in pydvl. Another example is in Mystic

If I understand your proposal correctly, this would mean that we handle convergence for all optimizers inside estimagic instead of expecting individual optimizers to do it.

The problem is that this is very hard to implement for wrapped optimizers. The only thing we could do for all optimizers is check convergece after every function evaluation. However, typically optimizers check convergence after every iteration. The difference is that one iteration typically entails multiple function evaluations and not all of them are expected to lead to a (substantial) improvement in the function value (exploration vs. exploitation). This a slow progress over a few function evaluations could lead to spurious convergence.

On a side not: This is the reason why an optimizer architecture where each optimizer just proposes a next step and then all objective evaluations are done in one main loop for all optimizers is hard to do (despite it's conceptual appeal).

Of course, we could only consider iterations that lead to an actual improvement in function values, but it does not eliminate all problems.

We can tackle this at some point but I see it as independent of the current proposal. After all, we will need a naming scheme for those objects.

[hmgaudecker]

I think I was just thinking of grouping related options in one object, instead of repeating "convergence" in every string!

[janosg]

I think I was just thinking of grouping related options in one object, instead of repeating "convergence" in every string!

Ah, we have the algorithm.with_stopping and algorithm.with_convergence copy constructors for that. They are described here

So far I did not propose to generally split the algo_options into three because this would mean that we become more different from scipy and it does not play well with creating configured instances of algorithms as we cannot introduce those namespaces there.

[hmgaudecker]

Great stuff!