Skip to content

run test_balancing_learner for all strategies #218

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

basnijholt
Copy link
Member

@basnijholt basnijholt commented Sep 18, 2019

@python-adaptive/core this reveals a regression in the LearnerND.

Should we merge this already (without a fix)?

also: @JornHoofwijk

@basnijholt
Copy link
Member Author

Something broke in between these two commits to learnerND.py
image

However, when I reverse the exact changes from ca8b1d5 in that state of the repo, it is still broken.

So seemingly it is not the code inside the LearnerND that causes the error.

@basnijholt
Copy link
Member Author

basnijholt commented Sep 18, 2019

test.py

#!/usr/bin/env python3
import sys
from functools import partial
import adaptive

f = lambda x: x[0]
learner = adaptive.BalancingLearner(
    [adaptive.LearnerND(f, bounds=[(-1, 1), (-1, 1)])], strategy="loss"
)
try: # fails in master
    adaptive.runner.simple(learner, goal=lambda l: l.learners[0].npoints > 20)
except:
    sys.exit(1)
sys.exit(0)

using the power of git bisect:

export PYTHONWARNINGS="ignore"
git bisect start ca8b1d5f70ca7fc05a2c8fddf813894a19fc3595 3f617442acdff18fcf4c2aceb725f7a93f0827c9
git bisect run ./test.py
running ./test.py
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[c233ceb9daecd7eaa32232551e2b3c5472ede815] change while loop to normal loop
running ./test.py
Bisecting: 2 revisions left to test after this (roughly 1 step)
[07f24b9ee02c72d811726c8dc76bd92ccd18b41e] add a test to ask for 0 points
running ./test.py
Bisecting: 0 revisions left to test after this (roughly 1 step)
[dc846e1513f7d28dcd5dd3fa9d1d7622a1b56222] add missing tell_pending
running ./test.py
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[fa616ef3537ac954138f2bf7fbc20cb1159e5ea3] fix asking for 0 points
running ./test.py
dc846e1513f7d28dcd5dd3fa9d1d7622a1b56222 is the first bad commit
commit dc846e1513f7d28dcd5dd3fa9d1d7622a1b56222
Author: Bas Nijholt <basnijholt@gmail.com>
Date:   Sun Mar 17 17:10:55 2019 +0100

    add missing tell_pending

So dc846e1 is the culprit introduced in #160.

@jbweston
Copy link
Contributor

We should probably add the test, but we can probably wait until #220 is merged, because that will replace LearnerND anyway.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants