Skip to content

Commit 8e4cb19

Browse files
jackgerritsataymano
jackgerrits
and
ataymano
authored
fix: fix #4669 by handling empty decision scores elements (#4670)
* fix: fix #4669 by handling empty decision scores elements * simplify test * ensure empty predictions do not affect num_labeled as well as loss * Update conditional_contextual_bandit.cc * Bounds check for explicit inclusion * Formatting --------- Co-authored-by: Alexey Taymanov <[email protected]>
1 parent 1675c6b commit 8e4cb19

10 files changed

+151
-4
lines changed

‎test/core.vwtest.json

+29
Original file line numberDiff line numberDiff line change
@@ -6073,5 +6073,34 @@
60736073
"depends_on": [
60746074
467
60756075
]
6076+
},
6077+
{
6078+
"id": 469,
6079+
"desc": "https://github.com/VowpalWabbit/vowpal_wabbit/issues/4669",
6080+
"vw_command": "--ccb_explore_adf --dsjson -d train-sets/issue4669.dsjson -f issue4669.model",
6081+
"diff_files": {
6082+
"stderr": "train-sets/ref/issue4669_train.stderr",
6083+
"stdout": "train-sets/ref/issue4669_train.stdout"
6084+
},
6085+
"input_files": [
6086+
"train-sets/issue4669.dsjson"
6087+
]
6088+
},
6089+
{
6090+
"id": 470,
6091+
"desc": "https://github.com/VowpalWabbit/vowpal_wabbit/issues/4669",
6092+
"vw_command": "--ccb_explore_adf --dsjson --all_slots_loss --epsilon 0 -t -i issue4669.model -t -d train-sets/issue4669.dsjson -p issue4669_test_pred.txt",
6093+
"diff_files": {
6094+
"stderr": "train-sets/ref/issue4669_test.stderr",
6095+
"stdout": "train-sets/ref/issue4669_test.stdout",
6096+
"issue4669_test_pred.txt": "train-sets/ref/issue4669_test_pred.txt"
6097+
},
6098+
"input_files": [
6099+
"train-sets/issue4669.dsjson",
6100+
"issue4669.model"
6101+
],
6102+
"depends_on": [
6103+
469
6104+
]
60766105
}
60776106
]

‎test/train-sets/issue4669.dsjson

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"c":{"_multi":[{"f":"1"},{"f":"2"}],"_slots":[{"_inc":[0,1]},{"_inc":[1]}]},"_outcomes":[{"_label_cost":1.0,"_a":[0,1],"_p":[0.5,0.5]},{"_label_cost":0.0,"_a":[1],"_p":[1]}]}

‎test/train-sets/ref/issue4669_test.stderr

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
only testing
2+
predictions = issue4669_test_pred.txt
3+
using no cache
4+
Reading datafile = train-sets/issue4669.dsjson
5+
num sources = 1
6+
Num weight bits = 18
7+
learning rate = 0.5
8+
initial_t = 1
9+
power_t = 0.5
10+
cb_type = mtr
11+
Enabled learners: gd, generate_interactions, scorer-identity, csoaa_ldf-rank, cb_adf, cb_explore_adf_greedy, cb_sample, shared_feature_merger, ccb_explore_adf
12+
Input label = CCB
13+
Output pred = DECISION_PROBS
14+
average since example example current current current
15+
loss last counter weight label predict features
16+
0.000000 0.000000 1 1.0 0:1,1:0 1,None 9
17+
18+
finished run
19+
number of examples = 1
20+
weighted example sum = 1.000000
21+
weighted label sum = 0.000000
22+
average loss = 0.000000
23+
total feature number = 9

‎test/train-sets/ref/issue4669_test.stdout

Whitespace-only changes.

‎test/train-sets/ref/issue4669_test_pred.txt

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
1:1,0:0
2+
3+

‎test/train-sets/ref/issue4669_train.stderr

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
final_regressor = issue4669.model
2+
using no cache
3+
Reading datafile = train-sets/issue4669.dsjson
4+
num sources = 1
5+
Num weight bits = 18
6+
learning rate = 0.5
7+
initial_t = 0
8+
power_t = 0.5
9+
cb_type = mtr
10+
Enabled learners: gd, generate_interactions, scorer-identity, csoaa_ldf-rank, cb_adf, cb_explore_adf_greedy, cb_sample, shared_feature_merger, ccb_explore_adf
11+
Input label = CCB
12+
Output pred = DECISION_PROBS
13+
average since example example current current current
14+
loss last counter weight label predict features
15+
1.000000 1.000000 1 1.0 0:1,1:0 0,1 12
16+
17+
finished run
18+
number of examples = 1
19+
weighted example sum = 1.000000
20+
weighted label sum = 0.000000
21+
average loss = 1.000000
22+
total feature number = 12

‎test/train-sets/ref/issue4669_train.stdout

Whitespace-only changes.

‎vowpalwabbit/core/src/decision_scores.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ void print_update(VW::workspace& all, const VW::multi_ex& slots, const VW::decis
2626
std::string delim;
2727
for (const auto& slot : decision_scores)
2828
{
29-
pred_ss << delim << slot[0].action;
29+
if (slot.empty()) { pred_ss << delim << "None"; }
30+
else { pred_ss << delim << slot[0].action; }
3031
delim = ",";
3132
}
3233
all.sd->print_update(*all.output_runtime.trace_message, all.passes_config.holdout_set_off,

‎vowpalwabbit/core/src/reductions/conditional_contextual_bandit.cc

+15-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "vw/core/reductions/conditional_contextual_bandit.h"
66

77
#include "vw/config/options.h"
8+
#include "vw/core/cb.h"
89
#include "vw/core/ccb_label.h"
910
#include "vw/core/ccb_reduction_features.h"
1011
#include "vw/core/constant.h"
@@ -213,8 +214,12 @@ void clear_pred_and_label(ccb_data& data)
213214
data.actions[data.action_with_label]->l.cb.costs.clear();
214215
}
215216

216-
// true if there exists at least 1 action in the cb multi-example
217-
bool has_action(VW::multi_ex& cb_ex) { return !cb_ex.empty(); }
217+
// true if there exists at least 2 examples (since there can only be up to 1
218+
// shared example), or the 0th example is not shared.
219+
bool has_action(VW::multi_ex& cb_ex)
220+
{
221+
return cb_ex.size() > 1 || (!cb_ex.empty() && !VW::ec_is_example_header_cb(*cb_ex[0]));
222+
}
218223

219224
// This function intentionally does not handle increasing the num_features of the example because
220225
// the output_example function has special logic to ensure the number of features is correctly calculated.
@@ -309,7 +314,11 @@ void build_cb_example(VW::multi_ex& cb_ex, VW::example* slot, const VW::ccb_labe
309314
// First time seeing this, initialize the vector with falses so we can start setting each included action.
310315
if (data.include_list.empty()) { data.include_list.assign(data.actions.size(), false); }
311316

312-
for (uint32_t included_action_id : explicit_includes) { data.include_list[included_action_id] = true; }
317+
for (uint32_t included_action_id : explicit_includes)
318+
{
319+
// The action may be included but not actually exist in the list of possible actions.
320+
if (included_action_id < data.actions.size()) { data.include_list[included_action_id] = true; }
321+
}
313322
}
314323

315324
// set the available actions in the cb multi-example
@@ -545,6 +554,9 @@ void update_stats_ccb(const VW::workspace& /* all */, shared_data& sd, const ccb
545554
if (outcome != nullptr)
546555
{
547556
num_labeled++;
557+
// It is possible for the prediction to be empty if there were no actions available at the time of taking the
558+
// slot decision. In this case it does not contribute to loss.
559+
if (preds[i].empty()) { continue; }
548560
if (i == 0 || data.all_slots_loss_report)
549561
{
550562
const float l = VW::get_cost_estimate(outcome->probabilities[VW::details::TOP_ACTION_INDEX], outcome->cost,

‎vowpalwabbit/core/tests/ccb_test.cc

+56
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,59 @@ TEST(Ccb, InsertInteractionsImplTest)
145145

146146
EXPECT_THAT(result, testing::ContainerEq(expected_after));
147147
}
148+
149+
TEST(Ccb, ExplicitIncludedActionsNonExistentAction)
150+
{
151+
auto vw = VW::initialize(vwtest::make_args("--ccb_explore_adf", "--quiet"));
152+
VW::multi_ex examples;
153+
examples.push_back(VW::read_example(*vw, "ccb shared |"));
154+
examples.push_back(VW::read_example(*vw, "ccb action |"));
155+
examples.push_back(VW::read_example(*vw, "ccb slot 0:10:10 10 |"));
156+
157+
vw->learn(examples);
158+
159+
auto& decision_scores = examples[0]->pred.decision_scores;
160+
EXPECT_EQ(decision_scores.size(), 1);
161+
EXPECT_EQ(decision_scores[0].size(), 0);
162+
vw->finish_example(examples);
163+
}
164+
165+
TEST(Ccb, NoAvailableActions)
166+
{
167+
auto vw = VW::initialize(vwtest::make_args("--ccb_explore_adf", "--quiet", "--all_slots_loss"));
168+
{
169+
VW::multi_ex examples;
170+
examples.push_back(VW::read_example(*vw, "ccb shared |"));
171+
examples.push_back(VW::read_example(*vw, "ccb action | a"));
172+
examples.push_back(VW::read_example(*vw, "ccb action | b"));
173+
examples.push_back(VW::read_example(*vw, "ccb slot 0:-1:0.5 0,1 |"));
174+
examples.push_back(VW::read_example(*vw, "ccb slot |"));
175+
176+
vw->learn(examples);
177+
178+
auto& decision_scores = examples[0]->pred.decision_scores;
179+
EXPECT_EQ(decision_scores.size(), 2);
180+
vw->finish_example(examples);
181+
}
182+
183+
{
184+
VW::multi_ex examples;
185+
examples.push_back(VW::read_example(*vw, "ccb shared |"));
186+
examples.push_back(VW::read_example(*vw, "ccb action | a"));
187+
examples.push_back(VW::read_example(*vw, "ccb action | b"));
188+
examples.push_back(VW::read_example(*vw, "ccb slot 0:-1:0.5 0,1 |"));
189+
// This time restrict slot 1 to only have action 0 available
190+
examples.push_back(VW::read_example(*vw, "ccb slot 0:-1:0.5 0 |"));
191+
192+
vw->predict(examples);
193+
194+
auto& decision_scores = examples[0]->pred.decision_scores;
195+
EXPECT_EQ(decision_scores.size(), 2);
196+
EXPECT_EQ(decision_scores[0].size(), 2);
197+
EXPECT_EQ(decision_scores[0][0].action, 0);
198+
EXPECT_EQ(decision_scores[0][1].action, 1);
199+
EXPECT_EQ(decision_scores[1].size(), 0);
200+
201+
vw->finish_example(examples);
202+
}
203+
}

0 commit comments

Comments
 (0)