Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

run test_balancing_learner for all strategies #218

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 <[email protected]>
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.

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

Successfully merging this pull request may close these issues.

2 participants