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

First attempt at softmax learner for cbadf #1839

Merged
merged 7 commits into from May 10, 2019

Conversation

Projects
None yet
4 participants
@adith387
Copy link
Contributor

commented Apr 11, 2019

  • Do scores from cb-adf.predict have to be negated before taking softmax? (i.e. does rest of vw treat lower scores as better?)
  • Is it correct to treat cb-adf.predict as the current regressor's scores?
  • Does DM handle shared features properly? Current implementation builds on DM since it is conceptually simpler than MTR.
  • When creating $k$ squared-loss-regression examples, the original multi-line example weights are backed up and restored. Do we also need to back-up and restore the original number of features in each example?
@JohnLangford

This comment has been minimized.

Copy link
Member

commented Apr 15, 2019

@adith387 status on this?

@JohnLangford

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

Ping?

@marco-rossi29

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

I've run --cb_type sm with a sample dataset from cmplx.

Issue to be addressed:

  1. Prediction prob are > 1 (see current predict column below)
  2. Average loss is not good. Maybe just a byproduct of issue 1
>vw --dsjson D:\data\logDownloader_v5\cmplx\cmplx_deepmerged_data_2018-10-01_2018-10-14.json.gz --cb_type sm --cb_adf -P 50000 -c
Num weight bits = 18
learning rate = 0.5
initial_t = 0
power_t = 0.5
using cache_file = D:\data\logDownloader_v5\cmplx\cmplx_deepmerged_data_2018-10-01_2018-10-14.json.gz.cache
ignoring text input in favor of cache input
num sources = 1
average  since         example        example  current  current  current
loss     last          counter         weight    label  predict features
-0.013296 -0.013296        50000        50000.0    known        4:10.6072...       73
-0.013022 -0.012749       100000       100000.0    known        0:14.4278...       54
-0.013974 -0.015878       150000       150000.0    known        0:17.1166...       69
-0.014888 -0.017628       200000       200000.0    known        0:17.6764...       57
-0.013458 -0.007738       250000       250000.0    known        0:22.8696...      156
-0.012211 -0.005979       300000       300000.0    known        0:12.4104...       90
-0.011255 -0.005517       350000       350000.0    known        2:25.4138...       56
-0.010141 -0.002342       400000       400000.0    known        0:24.7752...       93
-0.009829 -0.007330       450000       450000.0    known        2:33.932...       52
-0.009511 -0.006655       500000       500000.0    known        3:30.5777...       52
-0.009322 -0.007425       550000       550000.0    known        0:30.3612...       77
-0.008964 -0.005034       600000       600000.0    known        1:27.9163...       99

finished run
number of examples = 613086
weighted example sum = 613086.000000
weighted label sum = 0.000000
average loss = -0.008962
total feature number = 42089178

Using mtr gives predictions with a real pdf:

>vw --dsjson D:\data\logDownloader_v5\cmplx\cmplx_deepmerged_data_2018-10-01_2018-10-14.json.gz --cb_type mtr --cb_adf -P 50000 -c
Num weight bits = 18
learning rate = 0.5
initial_t = 0
power_t = 0.5
using cache_file = D:\data\logDownloader_v5\cmplx\cmplx_deepmerged_data_2018-10-01_2018-10-14.json.gz.cache
ignoring text input in favor of cache input
num sources = 1
average  since         example        example  current  current  current
loss     last          counter         weight    label  predict features
-0.016839 -0.016839        50000        50000.0    known        1:0.208558...       73
-0.015842 -0.014845       100000       100000.0    known        1:0.0445442...       54
-0.015125 -0.013690       150000       150000.0    known        5:0.0301438...       69
-0.015519 -0.016703       200000       200000.0    known        1:0.133496...       57
-0.014740 -0.011621       250000       250000.0    known        2:0.164838...      156
-0.014324 -0.012245       300000       300000.0    known        0:0.0599655...       90
-0.014063 -0.012497       350000       350000.0    known        0:0.0595418...       56
-0.013963 -0.013264       400000       400000.0    known        1:0.12498...       93
-0.014031 -0.014572       450000       450000.0    known        2:0.0144068...       52
-0.013754 -0.011267       500000       500000.0    known        2:0.0596526...       52
-0.013834 -0.014630       550000       550000.0    known        1:-0.0112924...       77
-0.014027 -0.016153       600000       600000.0    known        2:-0.0609681...       99

finished run
number of examples = 613086
weighted example sum = 613086.000000
weighted label sum = 0.000000
average loss = -0.013905
total feature number = 42089178

@adith387 How do you want to debug this?

@JohnLangford

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

I suspect at least some of this is a reporting issue rather than a learning dynamics issue.

{
if (prob <= 0.)
{
std::cout << "Probability " << prob << " is not possible, replacing with 1e-6. Fix your dataset. " << std::endl;

This comment has been minimized.

Copy link
@jackgerrits

jackgerrits May 2, 2019

Member

Error messages or warnings should be printed to cerr

This comment has been minimized.

Copy link
@adith387

adith387 May 2, 2019

Author Contributor

Made the following changes:
cb_adf.cc: Removed the safe_probability method. Replaced all calls to it to call GEN_CS::safe_probability instead. Having 2 safe_probability methods is not healthy.
gen_cs_example.cc::safe_probability prints error message to std::cout and needs to be fixed in the master branch.

@JohnLangford

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 2, 2019

Doesn't all.trace_message go to stderr though?

@JohnLangford

This comment has been minimized.

Copy link
Member

commented May 2, 2019

@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 8, 2019

@adith387 just letting you know I fixed the merge conflict so you'll need to pull before submitting any more commits.

@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 9, 2019

@adith387 we need a test in RunTests for this, do you need a hand with that?

@adith387

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

@adith387 we need a test in RunTests for this, do you need a hand with that?

Yes please!

@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Okay no problem, I can actually just check in a test for you. Would you be able to share me the hand crafted data set you made?

@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 10, 2019

I was unable to commit to this branch so I've created a PR on your fork adding the test: adith387#1. Once you merge that it will show up here

adith387 and others added some commits May 10, 2019

Merge pull request #1 from jackgerrits/adith/master
Add test for --cb_adf --cb_type sm
Merge pull request #2 from VowpalWabbit/master
Merging latest changes in vw master

@jackgerrits jackgerrits merged commit 45a8a7f into VowpalWabbit:master May 10, 2019

8 checks passed

LGTM analysis: C# No code changes detected
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
LGTM analysis: Java No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.08%) to 73.036%
Details
@jackgerrits

This comment has been minimized.

Copy link
Member

commented May 10, 2019

Thanks @adith387, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.