Visitar URL original
fix(grad_accum): preserve plain scalar summaries during minibatch merge · apple/axlearn@0c911d9 · GitHub
Skip to content

Commit 0c911d9

Browse files
Ralf Kaestnerwangkuiyi
authored andcommitted
fix(grad_accum): preserve plain scalar summaries during minibatch merge
GitOrigin-RevId: 9f87ad3
1 parent 52903f5 commit 0c911d9

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

‎axlearn/common/gradient_accumulation.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from axlearn.common import utils
1414
from axlearn.common.config import ConfigOr, maybe_instantiate
1515
from axlearn.common.metrics import MetricAccumulator
16+
from axlearn.common.summary import Summary
1617
from axlearn.common.update_transformation import ( # pytype: disable=pyi-error
1718
ForwardFn,
1819
ForwardOutputs,
@@ -288,7 +289,21 @@ def scan_body(
288289
# Since we summed during accumulation we take the average here to rescale.
289290
grads = jax.tree.map(lambda x: x / steps, grads)
290291
primal_out = jax.tree.map(lambda x: x / steps, primal_out)
291-
primal_out.output_collection.summaries.update(accumulator.summaries())
292+
293+
# Merge accumulator summaries with primal_out at the leaf level. For Summary
294+
# instances, use the accumulator's properly accumulated value. For plain scalars
295+
# keep primal_out's value. The accumulator maps non-Summary leaves to None.
296+
def _is_summary_leaf(x):
297+
return isinstance(x, Summary) or x is None
298+
299+
merged = jax.tree.map(
300+
lambda acc, pri: acc if isinstance(acc, Summary) else pri,
301+
accumulator.summaries(),
302+
primal_out.output_collection.summaries,
303+
is_leaf=_is_summary_leaf,
304+
)
305+
primal_out.output_collection.summaries.clear()
306+
primal_out.output_collection.summaries.update(merged)
292307
return primal_out, grads
293308

294309
def sequential_vmap(func: ForwardFn) -> ForwardFn:

‎axlearn/common/gradient_accumulation_test.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,5 +189,55 @@ def loss_fn(*, model_params, inputs) -> ForwardOutputs:
189189
)
190190

191191

192+
class TestMinibatchSummaryMerge(test_utils.TestCase):
193+
"""Test that `with_minibatch_steps` correctly merges Summary vs plain scalar summaries."""
194+
195+
@parameterized.named_parameters(
196+
("two_steps", 2),
197+
("four_steps", 4),
198+
)
199+
def test_summary_merge_with_plain_scalars(self, steps):
200+
"""Tests that Summary instances use accumulator values while plain scalars use averaging."""
201+
params = dict(w=jnp.asarray([0.0, 2.0, 2.0, -3.0]))
202+
203+
def loss_fn(*, model_params, inputs) -> ForwardOutputs:
204+
loss = -jax.nn.log_softmax(model_params["w"])[1]
205+
output_collection = new_output_collection()
206+
# A Summary instance — should be accumulated via MetricAccumulator.
207+
output_collection.summaries["weighted_metric"] = WeightedSummary(loss, jnp.array(1.0))
208+
# A plain scalar — should be accumulated via sum/divide-by-steps.
209+
output_collection.summaries["plain_scalar"] = loss
210+
del inputs
211+
return ForwardOutputs(loss=loss, aux={}, output_collection=output_collection)
212+
213+
forward_key, param_noise_key = jax.random.split(jax.random.PRNGKey(0), 2)
214+
inputs = dict(
215+
input_batch=jnp.ones((steps * 4, 8)),
216+
forward_key=forward_key,
217+
param_noise_key=param_noise_key,
218+
)
219+
220+
# Without minibatch (baseline).
221+
out_baseline = loss_fn(model_params=params, inputs=inputs)
222+
223+
# With minibatch.
224+
loss_fn_mb = gradient_accumulation.with_minibatch_steps(
225+
steps=steps, metric_accumulator=MetricAccumulator.default_config()
226+
)(loss_fn)
227+
out_mb = loss_fn_mb(model_params=params, inputs=inputs)
228+
229+
# The plain scalar should match (sum of identical values / steps == original value).
230+
test_utils.assert_allclose(
231+
out_baseline.output_collection.summaries["plain_scalar"],
232+
out_mb.output_collection.summaries["plain_scalar"],
233+
)
234+
# The WeightedSummary should be properly accumulated (mean stays the same,
235+
# weight == steps since each minibatch contributes weight=1).
236+
ws = out_mb.output_collection.summaries["weighted_metric"]
237+
self.assertIsInstance(ws, WeightedSummary)
238+
test_utils.assert_allclose(ws.mean, out_baseline.loss)
239+
test_utils.assert_allclose(ws.weight, jnp.array(float(steps)))
240+
241+
192242
if __name__ == "__main__":
193243
absltest.main()

0 commit comments

Comments
 (0)