Add loss_goal, npoints_goal, and an auto_goal function and use it in the runners

[basnijholt]

Description

Add more loss options to Runners to easily set common loss conditions.

For example:

learner = Learner1D(...)
runner = Runner(learner, npoints_goal=100)
# or
runner = Runner(learner, loss_goal=0.01)

learner = SequenceLearner(...)
runner = Runner(learner)  # stops when out of points

learner = IntegratorLearner(..., rtol=0.01)
runner = simple(learner)  # stops when reached tol

learner = BalancingLearner([list of IntegratorLearners])
runner = Runner(learner)  # stops when all reached tol

learner = Learner1D(...)
runner = Runner(learner)  # never stops

learner = SequenceLearner(...)
runner = Runner(learner, duration_goal=timedelta(hours=5))

Still one is also able to use auto_goal:

from adaptive.runner import auto_goal
learner = Learner2D(...)
runner = Runner(learner, auto_goal(loss=0.01))
runner = Runner(learner, auto_goal(npoints=100))
runner = Runner(learner, auto_goal(timedelta=timedelta(hours=1))

Checklist

• Fixed style issues using pre-commit run --all (first install using pip install pre-commit)
• pytest passed

Type of change

Check relevant option(s).

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• (Code) style fix or documentation update
• This change requires a documentation update

[akhmerov]

I like the idea of simplifying common use cases, and therefore I find the MR promising, however I think that the logic for interpreting the input is too obscure and would hurt readability. Instead we could introduce alternative arguments to the runner, e.g. Runner(..., time=...) etc.

[codecov-commenter]

Codecov Report

Merging #382 (b5f6f26) into main (ae44fae) will increase coverage by 0.18%.
The diff coverage is 82.44%.

@@            Coverage Diff             @@
##             main     #382      +/-   ##
==========================================
+ Coverage   77.73%   77.92%   +0.18%     
==========================================
  Files          37       37              
  Lines        5422     5508      +86     
  Branches      981      989       +8     
==========================================
+ Hits         4215     4292      +77     
- Misses       1061     1065       +4     
- Partials      146      151       +5     

Impacted Files	Coverage Δ
adaptive/runner.py	68.14% <61.11%> (-0.46%)	⬇️
adaptive/learner/data_saver.py	76.81% <85.71%> (+2.20%)	⬆️
adaptive/tests/test_runner.py	91.15% <97.91%> (+4.90%)	⬆️
adaptive/tests/test_average_learner.py	97.67% <100.00%> (ø)
adaptive/tests/test_balancing_learner.py	97.14% <100.00%> (ø)
adaptive/tests/test_learner1d.py	92.72% <100.00%> (ø)
adaptive/tests/test_learnernd.py	96.15% <100.00%> (ø)
adaptive/tests/test_learners.py	86.86% <100.00%> (ø)
adaptive/tests/test_pickling.py	88.67% <100.00%> (ø)
adaptive/tests/test_sequence_learner.py	100.00% <100.00%> (ø)
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[akhmerov]

I thought about it more, and I'm pretty sure that bundling all options into goal makes the code less readable and more error-prone. I think adding to the runner a handful of arguments that specify different termination conditions is the way to go.

There are examples where input interpretation depends on its shape, in particular a bunch of numpy API, but I don't know where as much of interpretation would depend on input types.

[basnijholt]

@akhmerov, I had a long discussion with @jbweston today and we settled on what I added in the description of this PR.

tl;dr, he agreed with you and now we are being explicit.

[akhmerov]

Thanks! A naming question: do we want to keep _goal in the parameter names? npoints seems clear enough. On the other hand timedelta is a type, duration would be clearer, and it should probably accept float-like numbers of seconds too. Likewise datetime could be end_time. auto_goal seems 👌.

[basnijholt]

I,

• renamed timedelta_goal to duration_goal,
• renamed datetime_goal to end_time_goal,
• allowed duration_goal to be a number, which indicates the number of seconds.

I think having the _goal explicit in the Runner aguments is clearer because it groups all of those arguments together.

[jbweston]

Nice feature!

There are a few minor fixups that could be applied, but other thatn that this is good to merge.

[akhmerov]

All looks good, thanks. I still think that _goal is superfluous in some of the new arguments like duration or end_time, but I leave this for you to decide.

[basnijholt]

Like I mentioned in #382 (comment) I think keeping the goal in the parameter name makes it clear that these parameters are grouped/belong together (@jbweston agreed in an offline discussion):

• goal
• loss_goal
• npoints_goal
• end_time_goal
• duration_goal

And it will be clear that they have a different meaning from the other parameters of e.g., the AsyncRunner:

• executor
• ntasks
• log
• shutdown_executor
• ioloop
• retries
• raise_if_retries_exceeded

[akhmerov]

I see, thanks for the explanation. I agree with the reasoning.