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: cb_adf learn_returns_prediction() incorrect or condition #2953

Merged
merged 2 commits into from Apr 19, 2021
Merged

fix: cb_adf learn_returns_prediction() incorrect or condition #2953

merged 2 commits into from Apr 19, 2021

Conversation

rajan-chari
Copy link
Member

CB_TYPE_DR || CB_TYPE_DM resolves to 0 || 1 which is equal to 1. This bug results in an extra call to predict() for CB_TYPE_DR.

@jackgerrits
Copy link
Member

jackgerrits commented Apr 16, 2021

Since this is a single commit PR (at the moment), the commit message needs to be prefixed with the type also. To resolve, either push another commit or update the existing commit to include fix: at the start.

Explanation from the workflow

          # Reason from action docs:
          # When using "Squash and merge" on a PR with only one commit, GitHub
          # will suggest using that commit message instead of the PR title for the
          # merge commit, and it's easy to commit this by mistake. Enable this option
          # to also validate the commit message for one commit PRs.

@jackgerrits jackgerrits merged commit 0928875 into VowpalWabbit:master Apr 19, 2021
jackgerrits added a commit to jackgerrits/vowpal_wabbit that referenced this pull request Apr 20, 2021
* ci: fix invalid job name (VowpalWabbit#2954)

* fix: cb_adf learn_returns_prediction (VowpalWabbit#2953)

Co-authored-by: olgavrou <olgavrou@gmail.com>

* fix: fix cost assignment in cats tree (VowpalWabbit#2900)

* ci: Use the base ref of a PR to compare against for benchmarks (VowpalWabbit#2955)

* fix!: cs_active now returns active_multiclass prediction (VowpalWabbit#2930)

* WIP

* Fix infinite recursion

* Fix include

* Remove debug print

* Remove C# test

* Remove cs piece

* formatting

* fix python

* Fix name

* Fix unused param

Co-authored-by: olgavrou <olgavrou@gmail.com>

Co-authored-by: Rajan <rajan.chari@yahoo.com>
Co-authored-by: olgavrou <olgavrou@gmail.com>
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.

None yet

3 participants