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

Active Learning #610

Merged
merged 1 commit into from Dec 1, 2021
Merged

Active Learning #610

merged 1 commit into from Dec 1, 2021

Conversation

y0ast
Copy link
Contributor

@y0ast y0ast commented Nov 11, 2021

A UB code style version of the AL notebook. There are several small open todos in the code, but for the majority we (Andreas+me) thought it's time to share and obtain some feedback on the approach.

This script obtains ~80-90% accuracy with ~150 CIFAR-10 samples.

Open todo:

  • A test following deterministic_test.py.
  • Follow pylint style from the repo
  • Add docstrings

Left for follow up PR: Proper LR schedule and early stopping - this will improve performance significantly.

Joint work with @BlackHC (Andreas).

@google-cla
Copy link

google-cla bot commented Nov 11, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Nov 11, 2021
@BlackHC
Copy link
Contributor

BlackHC commented Nov 11, 2021

@googlebot I consent.

🎉

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 11, 2021
Copy link
Member

@dusenberrymw dusenberrymw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating this! Left some comments. I would also strongly recommend adding script-level testing like you mentioned in the open TODO list so that we can iterate while keeping the script deterministically unchanged (when we want it to).

baselines/jft/active_learning.py Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/al_utils.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Outdated Show resolved Hide resolved
baselines/jft/al_utils.py Outdated Show resolved Hide resolved
baselines/jft/active_learning.py Show resolved Hide resolved
@y0ast
Copy link
Contributor Author

y0ast commented Nov 22, 2021

@dusenberrymw This should now have addressed your comments, contain docstrings and tests.

Copy link
Member

@dusenberrymw dusenberrymw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Joost & Andreas! This LGTM. I'll pull it in and merge.

Three comments/questions:

  • Have you both singed the CLA?
  • Can you squash the commits to a single commit?
  • Also, I believe the commit message for that single commit can be edited such that GitHub will recognize two co-authors if you're interested in having both of you attributed to the commit. I've never done it, but here's the guide.

Uses predictive entropy as acquisition function and a fixed amount of
learning steps, for now.

Co-authored-by: Andreas Kirsch <blackhc@gmail.com>
@y0ast
Copy link
Contributor Author

y0ast commented Nov 30, 2021

  1. yes, CLA is signed by Andreas/Me (see also: https://github.com/google/uncertainty-baselines/pull/610/checks?check_run_id=4370587110)
  2. Merged in latest main + squashed
  3. Added Andreas as Co-authored-by

@dustinvtran
Copy link
Member

dustinvtran commented Nov 30, 2021

LGTM as well! Excited to have the code in.

@dusenberrymw dusenberrymw self-requested a review December 1, 2021 00:15
@copybara-service copybara-service bot merged commit e7c5af6 into google:main Dec 1, 2021
@y0ast y0ast deleted the active_learning branch December 2, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants