Skip to content

Commit

Permalink
Fix segfault in CCB - MTR must clean up predictions allocated for cos…
Browse files Browse the repository at this point in the history
…t sensitive examples (#2111)

* Fix crash in ccb due to prediciton being left behind by mtr

* Add data files

* add ref files

* Remove onethread from the command line

* Remove empty line

* Fix c# unit tests: 1. remove dsjson test from c# suite as it is not supported, 2. add extra newline so it can be correctly parsed
  • Loading branch information
jackgerrits committed Oct 14, 2019
1 parent 914611a commit e245cb8
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cs/unittest/RunTests.tt
Expand Up @@ -21,7 +21,7 @@ var skipList = new[] { 13, 32, 39, 40, 41, 59, 60, 61, 66, 68, 90,
176, 177, //depend on shell scripts for input/output
14, 16, 17, 31, 33, 34,53, 101, 102, 103, 105, 106, 111, 112, // float delta
71, // --examples to test parser
143, 144, 146, 158, 189, // native json parsing
143, 144, 146, 158, 189, 202, // native json parsing
149, 152, 156, 193, 194, // bash script
188, // possibly float delta
195 //--onethread is a shell option, not available via library
Expand Down
8 changes: 8 additions & 0 deletions test/RunTests
Expand Up @@ -1753,4 +1753,12 @@ printf '3 |f a b c |e x y z\n2 |f a y c |e x\n' | {VW} --oaa 3 -q ef --audit
train-sets/ref/cbe_adf_softmax_biglambda.stderr
pred-sets/ref/cbe_adf_softmax_biglambda.predict
# Test 201: Test memory corruption issue in ccb_explore_adf where mtr was leaving a prediction behind
{VW} --ccb_explore_adf --ring_size 7 -d train-sets/ccb_reuse_small.data
train-sets/ref/ccb_reuse_small.stderr
# Test 202: Test memory corruption issue in ccb_explore_adf where mtr was leaving a prediction behind
{VW} --ccb_explore_adf --ring_size 20 --dsjson -d train-sets/ccb_reuse_medium.dsjson
train-sets/ref/ccb_reuse_medium.stderr
# Do not delete this line or the empty line above it
4 changes: 4 additions & 0 deletions test/train-sets/ccb_reuse_medium.dsjson
@@ -0,0 +1,4 @@
{"Version":"1","c":{"TShared":{"a=1":1,"b=0":1,"c=1":1},"_multi":[{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=3.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}}],"_slots":[{"Slate":{"c":1},"_inc":[0,1,2,3]},{"Slate":{"c":1},"_inc":[4,5,6]},{"Slate":{"c":1},"_inc":[7,8]}]},"_outcomes":[{"_id":"ac32c0fc-f895-429d-9063-01c996432f791249622271","_label_cost":0,"_a":[0,1,2,3],"_p":[0.25,0.25,0.25,0.25],"_o":[0]},{"_id":"b64a5e7d-6e76-4d66-98fe-dc214e675ff81249622271","_label_cost":0,"_a":[4,5,6],"_p":[0.333333,0.333333,0.333333],"_o":[0]},{"_id":"a3a29e41-d903-4fbe-b624-11632733cf6f1249622271","_label_cost":0,"_a":[7,8],"_p":[0.5,0.5],"_o":[0]}],"VWState":{"m":"N/A"}}
{"Version":"1","c":{"TShared":{"a=1":1,"b=0":1,"c=1":1},"_multi":[{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=3.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}}],"_slots":[{"Slate":{"c":1},"_inc":[0,1,2,3]},{"Slate":{"c":1},"_inc":[4,5,6]},{"Slate":{"c":1},"_inc":[7,8]}]},"_outcomes":[{"_id":"556e7e48-071c-4a6b-8e27-309562b0ceb81249622271","_label_cost":-0.0917649,"_a":[0,1,2,3],"_p":[0.25,0.25,0.25,0.25],"_o":[-0.0917649]},{"_id":"65834b94-3ac6-4887-bf0b-b773c72613d91249622271","_label_cost":-0.0917649,"_a":[6,5,4],"_p":[0.333333,0.333333,0.333333],"_o":[-0.0917649]},{"_id":"1091eed1-7909-4e71-8de1-d6ed2cde7f4f1249622271","_label_cost":-0.0917649,"_a":[7,8],"_p":[0.5,0.5],"_o":[-0.0917649]}],"VWState":{"m":"N/A"}}
{"Version":"1","c":{"TShared":{"a=0":1,"b=0":1,"c=1":1},"_multi":[{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=3.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}}],"_slots":[{"Slate":{"c":1},"_inc":[0,1,2,3]},{"Slate":{"c":1},"_inc":[4,5,6]},{"Slate":{"c":1},"_inc":[7,8]}]},"_outcomes":[{"_id":"f1d4854d-e1b5-422a-bc7f-3ae4e42b1a841249622271","_label_cost":-0.025595,"_a":[2,1,0,3],"_p":[0.25,0.25,0.25,0.25],"_o":[-0.025595]},{"_id":"8ceef1a6-8927-4632-8621-6992afd8a03f1249622271","_label_cost":-0.025595,"_a":[4,5,6],"_p":[0.333333,0.333333,0.333333],"_o":[-0.025595]},{"_id":"b0797efc-a6a3-49fa-bb0d-df9f1f6c85a41249622271","_label_cost":-0.025595,"_a":[7,8],"_p":[0.5,0.5],"_o":[-0.025595]}],"VWState":{"m":"N/A"}}
{"Version":"1","c":{"TShared":{"a=1":1,"b=1":1,"c=1":1},"_multi":[{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=3.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}},{"TAction":{"value=2.000000":1}},{"TAction":{"value=0.000000":1}},{"TAction":{"value=1.000000":1}}],"_slots":[{"Slate":{"c":1},"_inc":[0,1,2,3]},{"Slate":{"c":1},"_inc":[4,5,6]},{"Slate":{"c":1},"_inc":[7,8]}]},"_outcomes":[{"_id":"97a2d580-53aa-4586-8620-ad16dc563f081249622271","_label_cost":-0.0230896,"_a":[0,1,2,3],"_p":[0.25,0.25,0.25,0.25],"_o":[-0.0230896]},{"_id":"6d945b21-d479-4a7a-8958-724d5032e22a1249622271","_label_cost":-0.0230896,"_a":[6,5,4],"_p":[0.333333,0.333333,0.333333],"_o":[-0.0230896]},{"_id":"763223f7-8ceb-41ef-a36b-73a25af75ca71249622271","_label_cost":-0.0230896,"_a":[7,8],"_p":[0.5,0.5],"_o":[-0.0230896]}],"VWState":{"m":"N/A"}}
20 changes: 20 additions & 0 deletions test/train-sets/ccb_reuse_small.data
@@ -0,0 +1,20 @@
ccb shared |User b
ccb action |Action d
ccb action |Action e
ccb slot 0:0:0.2 |Slot h

ccb shared |User b
ccb action |Action d
ccb action |Action e
ccb slot 1:0:0.2 |Slot h

ccb shared |User c
ccb action |Action d
ccb action |Action e
ccb slot 1:0:0.25 |Slot i

ccb shared |User a
ccb action |Action d
ccb action |Action e
ccb slot 1:0:0.2 |Slot h

19 changes: 19 additions & 0 deletions test/train-sets/ref/ccb_reuse_medium.stderr
@@ -0,0 +1,19 @@
Num weight bits = 18
learning rate = 0.5
initial_t = 0
power_t = 0.5
using no cache
Reading datafile = train-sets/ccb_reuse_medium.dsjson
num sources = 1
average since example example current current current
loss last counter weight label predict features
0.000000 0.000000 1 1.0 0:0.000000,4:0.000000,... 0,4,7,... 28
-0.137647 -0.275295 2 2.0 0:-0.091765,6:-0.091765,... 0,6,7,... 28
-0.150397 -0.163148 4 4.0 0:-0.023090,6:-0.023090,... 0,6,7,... 28

finished run
number of examples = 4
weighted example sum = 4.000000
weighted label sum = 0.000000
average loss = -0.150397
total feature number = 112
19 changes: 19 additions & 0 deletions test/train-sets/ref/ccb_reuse_small.stderr
@@ -0,0 +1,19 @@
Num weight bits = 18
learning rate = 0.5
initial_t = 0
power_t = 0.5
using no cache
Reading datafile = train-sets/ccb_reuse_small.data
num sources = 1
average since example example current current current
loss last counter weight label predict features
0.000000 0.000000 1 1.0 0:0.000000 0 8
0.000000 0.000000 2 2.0 1:0.000000 1 8
0.000000 0.000000 4 4.0 1:0.000000 1 8

finished run
number of examples = 4
weighted example sum = 4.000000
weighted label sum = 0.000000
average loss = 0.000000
total feature number = 32
4 changes: 4 additions & 0 deletions vowpalwabbit/cb_adf.cc
Expand Up @@ -41,6 +41,7 @@ struct cb_adf
v_array<COST_SENSITIVE::label> _prepped_cs_labels;

action_scores _a_s; // temporary storage for mtr and sm
action_scores _a_s_mtr_cs; // temporary storage for mtr cost sensitive example
action_scores _prob_s; // temporary storage for sm; stores softmax values
v_array<uint32_t> _backup_nf; // temporary storage for sm; backup for numFeatures in examples
v_array<float> _backup_weights; // temporary storage for sm; backup for weights in examples
Expand Down Expand Up @@ -93,6 +94,7 @@ struct cb_adf
_prob_s.delete_v();

_a_s.delete_v();
_a_s_mtr_cs.delete_v();
_gen_cs.pred_scores.costs.delete_v();
}

Expand Down Expand Up @@ -267,11 +269,13 @@ void cb_adf::learn_MTR(multi_learner& base, multi_ex& examples)
examples[_gen_cs.mtr_example]->weight *= 1.f / clipped_p *
((float)_gen_cs.event_sum / (float)_gen_cs.action_sum);

std::swap(_gen_cs.mtr_ec_seq[0]->pred.a_s, _a_s_mtr_cs);
// TODO!!! cb_labels are not getting properly restored (empty costs are dropped)
GEN_CS::call_cs_ldf<true>(
base, _gen_cs.mtr_ec_seq, _cb_labels, _cs_labels, _prepped_cs_labels, _offset);
examples[_gen_cs.mtr_example]->num_features = nf;
examples[_gen_cs.mtr_example]->weight = old_weight;
std::swap(_gen_cs.mtr_ec_seq[0]->pred.a_s, _a_s_mtr_cs);
std::swap(examples[0]->pred.a_s, _a_s);
}

Expand Down

0 comments on commit e245cb8

Please sign in to comment.