Skip to content

Commit 1637cfc

Browse files
authored
fix(c#): retain SimpleLabel importance weight when building examples (#4949)
SimpleLabelUpdateExample wrote the importance weight to example::weight, but that field is not where a simple label's weight lives. VW::setup_example -- which VowpalWabbitExampleBuilder.CreateExample runs to finalize the example -- reassigns it: ae->weight = lbl_parser.get_weight(ae->l, ae->ex_reduction_features); and for simple labels get_weight returns simple_label_reduction_features::weight, which ApplyLabel never set. So the weight was overwritten with its default of 1 between ApplyLabel and CreateExample, and SimpleLabelReadFromExample -- which reads the reduction feature, and was correct all along -- reported 1. Reported in #4945: a label built as { Label = 7, Weight = 4, Initial = 3 } reads back as 7 1 3. StringLabel is unaffected because the text parser writes simple_red_features.weight directly, which is why it round-trips correctly and made the two paths disagree. The consequence is worse than a bad round-trip: because ex->weight is what the learner consumes, an importance weight supplied through SimpleLabel never reached training at all -- every such example trained as though weighted 1.0. Set the reduction feature, which is the source of truth and survives setup_example, and keep assigning ex->weight so the value is also correct for callers that never run setup_example. Verified by driving the same native entry points the builder uses (SimpleLabelUpdateExample -> VW::setup_example -> SimpleLabelReadFromExample): before the change the harness reports "label=7 weight=1 initial=3", matching the report exactly; after it reports "label=7 weight=4 initial=3" with ex->weight=4. Adds two regression tests mirroring the reporter's: one that the weight round-trips, and one that it actually affects training. Closes #4945 Claude-Session: https://claude.ai/code/session_01EVprwZHP4KXAhsF6JGXK9k
1 parent 01fa96a commit 1637cfc

2 files changed

Lines changed: 88 additions & 4 deletions

File tree

cs/unittest/TestLabels.cs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,81 @@ public void TestStringLabels()
8181
}
8282
}
8383

84+
85+
/// <summary>
86+
/// Regression test for #4945. SimpleLabelUpdateExample wrote the importance weight to
87+
/// example::weight, but VW::setup_example -- which the builder runs inside CreateExample --
88+
/// reassigns that field from the simple-label reduction features, so the weight was
89+
/// overwritten with the default of 1 before anything could read it back. StringLabel was
90+
/// unaffected because the text parser writes the reduction feature directly.
91+
/// </summary>
92+
[TestMethod]
93+
[TestCategory("Vowpal Wabbit")]
94+
public void TestSimpleLabelRetainsWeight()
95+
{
96+
using (var vw = new VowpalWabbit("--quiet"))
97+
using (var builder = new VowpalWabbitExampleBuilder(vw))
98+
{
99+
builder.ApplyLabel(new SimpleLabel { Label = 7f, Weight = 4f, Initial = 3f });
100+
101+
using (var example = builder.CreateExample())
102+
{
103+
var read = (SimpleLabel)example.Label;
104+
Assert.AreEqual(7f, read.Label, "label");
105+
Assert.AreEqual(4f, read.Weight, "weight");
106+
Assert.AreEqual(3f, read.Initial, "initial");
107+
}
108+
}
109+
}
110+
111+
/// <summary>
112+
/// The consequence of the above: a weight that never reaches the learner trains as though
113+
/// it were 1.0, so these two produced identical predictions before the fix.
114+
/// </summary>
115+
[TestMethod]
116+
[TestCategory("Vowpal Wabbit")]
117+
public void TestSimpleLabelWeightAffectsTraining()
118+
{
119+
Func<ILabel, float> train = label =>
120+
{
121+
using (var vw = new VowpalWabbit("--link logistic --loss_function logistic -b 18 --quiet"))
122+
{
123+
using (var builder = new VowpalWabbitExampleBuilder(vw))
124+
{
125+
using (var ns = builder.AddNamespace('a'))
126+
{
127+
ns.AddFeature(vw.HashFeature("x", vw.HashSpace("a")), 1f);
128+
}
129+
130+
builder.ApplyLabel(label);
131+
132+
using (var example = builder.CreateExample())
133+
{
134+
vw.Learn(example);
135+
}
136+
}
137+
138+
using (var scoreBuilder = new VowpalWabbitExampleBuilder(vw))
139+
{
140+
using (var ns = scoreBuilder.AddNamespace('a'))
141+
{
142+
ns.AddFeature(vw.HashFeature("x", vw.HashSpace("a")), 1f);
143+
}
144+
145+
using (var scored = scoreBuilder.CreateExample())
146+
{
147+
return vw.Predict(scored, VowpalWabbitPredictionType.Scalar);
148+
}
149+
}
150+
}
151+
};
152+
153+
var heavy = train(new SimpleLabel { Label = 1f, Weight = 4f });
154+
var light = train(new SimpleLabel { Label = 1f, Weight = 0.05f });
155+
156+
Assert.AreNotEqual(heavy, light,
157+
"importance weight had no effect on training: both behaved as weight 1.0");
158+
}
84159
}
85160

86161
public class SimpleContext

cs/vw.net.native/vw.net.labels.cc

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,21 @@ API void SimpleLabelUpdateExample(vw_net_native::workspace_context* workspace, V
2020
auto* ld = &ex->l.simple;
2121
ld->label = label;
2222

23-
if (maybe_weight) { ex->weight = *maybe_weight; }
24-
25-
if (maybe_initial)
23+
// The importance weight lives in the simple-label reduction features, not on the example.
24+
// VW::setup_example assigns ex->weight from lbl_parser.get_weight(), which for simple labels
25+
// reads simple_label_reduction_features::weight -- so writing only ex->weight here was silently
26+
// discarded when the builder finalized the example, leaving the default weight of 1.
27+
// Set both: the reduction feature is the source of truth and survives setup_example, while
28+
// ex->weight keeps the value correct for callers that never run setup_example.
29+
if (maybe_weight || maybe_initial)
2630
{
2731
auto& red_fts = ex->ex_reduction_features.template get<VW::simple_label_reduction_features>();
28-
red_fts.initial = *maybe_initial;
32+
if (maybe_weight)
33+
{
34+
red_fts.weight = *maybe_weight;
35+
ex->weight = *maybe_weight;
36+
}
37+
if (maybe_initial) { red_fts.initial = *maybe_initial; }
2938
}
3039

3140
VW::count_label(*workspace->vw->sd, ld->label);

0 commit comments

Comments
 (0)