Closed janosg closed 1 year ago
Merging #446 (bf07d2a) into main (91ffcb8) will increase coverage by
0.05%
. The diff coverage is99.69%
.
@@ Coverage Diff @@
## main #446 +/- ##
==========================================
+ Coverage 92.99% 93.05% +0.05%
==========================================
Files 247 250 +3
Lines 18482 18597 +115
==========================================
+ Hits 17188 17305 +117
+ Misses 1294 1292 -2
Impacted Files | Coverage Δ | |
---|---|---|
...sts/optimization/subsolvers/test_gqtpar_lambdas.py | 100.00% <ø> (ø) |
|
...optimization/tranquilo/test_acceptance_decision.py | 100.00% <ø> (ø) |
|
tests/optimization/tranquilo/test_rho_noise.py | 100.00% <ø> (ø) |
|
src/estimagic/optimization/tranquilo/options.py | 99.18% <98.92%> (-0.82%) |
:arrow_down: |
...agic/optimization/tranquilo/acceptance_decision.py | 96.87% <100.00%> (ø) |
|
...c/optimization/tranquilo/acceptance_sample_size.py | 95.45% <100.00%> (ø) |
|
...timagic/optimization/tranquilo/aggregate_models.py | 97.67% <100.00%> (+10.17%) |
:arrow_up: |
...imagic/optimization/tranquilo/estimate_variance.py | 100.00% <100.00%> (ø) |
|
.../estimagic/optimization/tranquilo/filter_points.py | 79.83% <100.00%> (+0.16%) |
:arrow_up: |
src/estimagic/optimization/tranquilo/fit_models.py | 84.51% <100.00%> (-1.85%) |
:arrow_down: |
... and 10 more |
... and 4 files with indirect coverage changes
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Problems
target_sample_size
depends on themodel_type
; The defaultmodel_type
depends on thefunctype
. But themodel_type
can also be overwritten by the user. If so, we need to check that the user specified a valid model type before we can create the defaulttarget_sample_size
. I.e. everything is entangled and the order of processing stuff has to be chosen very carefully.Solution
options.py
; If default values depend on other arguments, we implementgetter_functions
, e.g.get_default_radius_options(x)
options.py
; In particular, all the logic related to combining default options and user options is inprocess_arguments.py
.tranquilo
function only callsprocess_arguments
and then calls_internal_tranquilo
with the processed arguments. This removes all namespace cluttering. A small trick to avoid long lists of arguments, the outertranquilo
functions only uses*args, **kwargs
as arguments andfunctools.wraps(process_arguments
) to get the right signature and docstring.Alternatives
We could use dags to figure out the order of everything, but we currently do not have dags as a dependency and we don't need it anywhere else. In any case, the current PR gets us closer to a situation where we could use dags.
To-Do
_internal_tranquilo
_new_tranquilo
to_tranquilo
RadiusOptions.initial_radius
a mandatory argument without default valueoptions.py
. Example:SubsolverOptions
that contains everything thedefault_options
dict insolve_subproblem
containsget_component
sucht that the user_options can be NamedTuples on which we call_asdict
solve_subproblem
instead of defining a dict thereaggregate_models
intoget_default_aggregator
if possible.Discuss
AcceptanceOptions
into individual options inacceptance_decision.py
.