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

fix: stash and restore prediction when calling learn during learn_returns_prediction==false calls #4632

Merged
merged 12 commits into from
Aug 31, 2023

Conversation

jackgerrits
Copy link
Member

@jackgerrits jackgerrits commented Aug 15, 2023

  • Fixes Igl's usage of the prediction in learn
  • Causes one less call to predict for cats which means the PRNG sequence changes and so do the predictions.
  • Seems that expreplay used to be returning the final prediction that was coming out of GD in its learn call. This fixes that to be the initial predict.

30.00431,0.00625
0.51225615,0.40625
2.3816023,0.40625
25.271912,0.00625
Copy link
Member

@ataymano ataymano Aug 22, 2023

Choose a reason for hiding this comment

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

numbers here are crazily different - maybe this test can be reconsidered (in next PRs)


finished run
number of examples per pass = 200
passes used = 8
weighted example sum = 1600.000000
weighted label sum = 728.000000
average loss = 0.047746
average loss = 0.455000
Copy link
Collaborator

Choose a reason for hiding this comment

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

so loss went up?

Copy link
Member Author

Choose a reason for hiding this comment

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

Previous loss was cheating since it was using predictions that were coming out of learn.

@ataymano ataymano merged commit 66d5572 into VowpalWabbit:master Aug 31, 2023
115 checks passed
@ataymano ataymano deleted the learn_returns_pred_fix branch August 31, 2023 14:50
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.

3 participants