Skip to content

Commit e975e9c

Browse files
WhiteSwan1claude
andcommitted
[review] SID: address tiankongdeguiji review on PR alibaba#538
- Metrics: switch the shared reconstruction metric to torchmetrics.MeanSquaredError (correct (preds, target) aggregation vs the biased mean-of-batch-means a MeanMetric gave), and add a UniqueRatio metric class (tzrec/metrics/unique_ratio.py) for codebook coverage instead of a MeanMetric fed a hand-computed ratio. BaseSidModel._update_unique_sid_ratio now delegates to it; the empty-batch guard + DDP reduction live in the metric. The RQ-VAE train-path mse moves to MeanSquaredError too. - __init__.py: revert tzrec/modules/sid_generation/__init__.py to a bare package marker (no re-exports) and import ResidualKMeansQuantizer from its submodule in sid_rqkmeans, per "avoid adding to __init__.py". Tests: tzrec/metrics/unique_ratio_test.py (ratio, empty no-op, mean over batches); sid_model_test verifies the delegation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f568de0 commit e975e9c

7 files changed

Lines changed: 115 additions & 57 deletions

File tree

tzrec/metrics/unique_ratio.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# Copyright (c) 2026, Alibaba Group;
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
12+
import torch
13+
from torchmetrics import Metric
14+
15+
16+
class UniqueRatio(Metric):
17+
"""Codebook-coverage metric: mean of per-batch (unique rows / batch size).
18+
19+
Each ``update`` counts the unique rows of a ``(B, n_layers)`` semantic-ID
20+
code tensor and accumulates the per-batch ratio; ``compute`` returns the
21+
running mean. Empty batches (``B == 0``, e.g. an empty final DDP/TorchRec
22+
shard) are skipped. States reduce by ``sum`` across ranks.
23+
"""
24+
25+
higher_is_better = True
26+
is_differentiable = False
27+
28+
def __init__(self, **kwargs) -> None:
29+
super().__init__(**kwargs)
30+
self.add_state("ratio_sum", default=torch.tensor(0.0), dist_reduce_fx="sum")
31+
self.add_state("count", default=torch.tensor(0.0), dist_reduce_fx="sum")
32+
33+
def update(self, codes: torch.Tensor) -> None:
34+
"""Accumulate the unique-ratio of one batch of codes.
35+
36+
Args:
37+
codes (Tensor): semantic-ID codes, shape (B, n_layers).
38+
"""
39+
batch_size = codes.shape[0]
40+
if batch_size == 0:
41+
return
42+
unique = torch.unique(codes, dim=0).shape[0]
43+
self.ratio_sum += unique / batch_size
44+
self.count += 1
45+
46+
def compute(self) -> torch.Tensor:
47+
"""Mean per-batch unique ratio (NaN before any non-empty update)."""
48+
return self.ratio_sum / self.count

tzrec/metrics/unique_ratio_test.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Copyright (c) 2026, Alibaba Group;
2+
# Licensed under the Apache License, Version 2.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# http://www.apache.org/licenses/LICENSE-2.0
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
12+
import unittest
13+
14+
import torch
15+
16+
from tzrec.metrics.unique_ratio import UniqueRatio
17+
18+
19+
class UniqueRatioTest(unittest.TestCase):
20+
def test_known_duplicates(self) -> None:
21+
metric = UniqueRatio()
22+
# 3 unique rows out of 4 -> 0.75.
23+
metric.update(torch.tensor([[1, 2], [1, 2], [3, 4], [5, 6]]))
24+
self.assertAlmostEqual(metric.compute().item(), 0.75, places=6)
25+
26+
def test_empty_batch_skipped(self) -> None:
27+
metric = UniqueRatio()
28+
metric.update(torch.empty(0, 3, dtype=torch.long))
29+
self.assertEqual(metric.count.item(), 0.0)
30+
self.assertTrue(torch.isnan(metric.compute()))
31+
32+
def test_mean_over_batches(self) -> None:
33+
metric = UniqueRatio()
34+
metric.update(torch.tensor([[1, 1], [1, 1]])) # 1/2 = 0.5
35+
metric.update(torch.tensor([[1, 1], [2, 2]])) # 2/2 = 1.0
36+
self.assertAlmostEqual(metric.compute().item(), 0.75, places=6) # mean
37+
38+
39+
if __name__ == "__main__":
40+
unittest.main()

tzrec/models/sid_model.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
from tzrec.datasets.utils import BASE_DATA_GROUP, Batch
2020
from tzrec.features.feature import BaseFeature
21+
from tzrec.metrics.unique_ratio import UniqueRatio
2122
from tzrec.models.model import BaseModel
2223
from tzrec.protos.model_pb2 import ModelConfig
2324

@@ -100,8 +101,8 @@ def init_metric(self) -> None:
100101
``unique_sid_ratio``: codebook coverage = unique SIDs / batch size.
101102
Subclasses call ``super().init_metric()`` then add their extras.
102103
"""
103-
self._metric_modules["mse"] = torchmetrics.MeanMetric()
104-
self._metric_modules["unique_sid_ratio"] = torchmetrics.MeanMetric()
104+
self._metric_modules["mse"] = torchmetrics.MeanSquaredError()
105+
self._metric_modules["unique_sid_ratio"] = UniqueRatio()
105106

106107
def update_train_metric(
107108
self,
@@ -116,13 +117,12 @@ def update_train_metric(
116117
return
117118

118119
def _update_unique_sid_ratio(self, codes: torch.Tensor) -> None:
119-
"""Update the codebook-coverage metric (unique SIDs / batch size).
120+
"""Update the codebook-coverage metric.
121+
122+
The unique-ratio math (and the empty-batch guard) lives in
123+
:class:`~tzrec.metrics.unique_ratio.UniqueRatio`.
120124
121125
Args:
122126
codes (Tensor): semantic-ID codes, shape (B, n_layers).
123127
"""
124-
B = codes.shape[0]
125-
if B == 0: # empty final shard under DDP/TorchRec
126-
return
127-
unique_sids = torch.unique(codes, dim=0).shape[0]
128-
self._metric_modules["unique_sid_ratio"].update(unique_sids / B)
128+
self._metric_modules["unique_sid_ratio"].update(codes)

tzrec/models/sid_model_test.py

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,30 @@
1212
import unittest
1313

1414
import torch
15-
import torchmetrics
1615

16+
from tzrec.metrics.unique_ratio import UniqueRatio
1717
from tzrec.models.sid_model import BaseSidModel
1818

1919

2020
class UpdateUniqueSidRatioTest(unittest.TestCase):
21-
"""Unit-test the codebook-coverage metric helper.
21+
"""Verify ``_update_unique_sid_ratio`` delegates to the UniqueRatio metric.
2222
23-
``_update_unique_sid_ratio`` is pure tensor logic over
24-
``self._metric_modules`` (no proto dependency), so it is testable without
25-
a full ``BaseSidModel`` config — full model coverage waits on the concrete
26-
subclasses in the follow-up PRs.
23+
The unique-ratio math itself is covered in
24+
``tzrec/metrics/unique_ratio_test.py``; here we only check the wiring (no
25+
proto dependency, so it is testable without a full ``BaseSidModel`` config).
2726
"""
2827

2928
def _bare_model(self) -> BaseSidModel:
3029
# Bypass __init__ (which needs a pipeline config); only the metric
3130
# module the helper touches needs to exist.
3231
model = BaseSidModel.__new__(BaseSidModel)
33-
model._metric_modules = {"unique_sid_ratio": torchmetrics.MeanMetric()}
32+
model._metric_modules = {"unique_sid_ratio": UniqueRatio()}
3433
return model
3534

3635
def test_empty_batch_is_noop(self) -> None:
3736
model = self._bare_model()
3837
model._update_unique_sid_ratio(torch.empty(0, 3, dtype=torch.long))
39-
# The B == 0 guard returns early -> no sample recorded.
40-
self.assertEqual(model._metric_modules["unique_sid_ratio"].weight.item(), 0.0)
38+
self.assertEqual(model._metric_modules["unique_sid_ratio"].count.item(), 0.0)
4139

4240
def test_ratio_on_known_duplicates(self) -> None:
4341
model = self._bare_model()

tzrec/models/sid_rqkmeans.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727
from tzrec.datasets.utils import Batch
2828
from tzrec.features.feature import BaseFeature
2929
from tzrec.models.sid_model import BaseSidModel
30-
from tzrec.modules.sid_generation import ResidualKMeansQuantizer
3130
from tzrec.modules.sid_generation.kmeans import recon_diagnostics
31+
from tzrec.modules.sid_generation.residual_kmeans_quantizer import (
32+
ResidualKMeansQuantizer,
33+
)
3234
from tzrec.protos.model_pb2 import ModelConfig
3335
from tzrec.utils import config_util
3436
from tzrec.utils.logging_util import logger
@@ -255,11 +257,15 @@ def update_metric(
255257
losses (dict, optional): a dict of loss.
256258
"""
257259
if "input_embedding" in predictions:
258-
mse, rel = recon_diagnostics(
260+
_, rel = recon_diagnostics(
259261
predictions["input_embedding"],
260262
predictions["quantized"],
261263
)
262-
self._metric_modules["mse"].update(mse)
264+
# MeanSquaredError aggregates (preds, target) itself; rel_loss has
265+
# no torchmetrics equivalent so it stays a MeanMetric.
266+
self._metric_modules["mse"].update(
267+
predictions["quantized"], predictions["input_embedding"]
268+
)
263269
self._metric_modules["rel_loss"].update(rel)
264270

265271
self._update_unique_sid_ratio(predictions["codes"])

tzrec/models/sid_rqvae.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ def init_metric(self) -> None:
327327
# is intentionally eval-only: torch.unique(codes, dim=0).shape[0]
328328
# forces a GPU->host sync every step, and codebook coverage is a
329329
# diagnostic, not a training signal.
330-
self._train_metric_modules["mse"] = torchmetrics.MeanMetric()
330+
self._train_metric_modules["mse"] = torchmetrics.MeanSquaredError()
331331

332332
def update_train_metric(
333333
self,
@@ -342,8 +342,7 @@ def update_train_metric(
342342
"""
343343
if "x_hat" in predictions:
344344
embedding = self._extract_feature(batch)
345-
mse = F.mse_loss(predictions["x_hat"], embedding, reduction="mean")
346-
self._train_metric_modules["mse"].update(mse)
345+
self._train_metric_modules["mse"].update(predictions["x_hat"], embedding)
347346

348347
def update_metric(
349348
self,
@@ -360,7 +359,6 @@ def update_metric(
360359
"""
361360
if "x_hat" in predictions:
362361
embedding = self._extract_feature(batch)
363-
mse = F.mse_loss(predictions["x_hat"], embedding, reduction="mean")
364-
self._metric_modules["mse"].update(mse)
362+
self._metric_modules["mse"].update(predictions["x_hat"], embedding)
365363

366364
self._update_unique_sid_ratio(predictions["codes"])

tzrec/modules/sid_generation/__init__.py

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,3 @@
88
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
99
# See the License for the specific language governing permissions and
1010
# limitations under the License.
11-
12-
from tzrec.modules.sid_generation.kmeans import (
13-
KMeansLayer,
14-
)
15-
from tzrec.modules.sid_generation.residual_kmeans_quantizer import (
16-
ResidualKMeansQuantizer,
17-
)
18-
from tzrec.modules.sid_generation.residual_quantizer import (
19-
ResidualQuantizer,
20-
)
21-
from tzrec.modules.sid_generation.residual_vector_quantizer import (
22-
ResidualVectorQuantizer,
23-
)
24-
from tzrec.modules.sid_generation.types import (
25-
QuantizeForwardMode,
26-
QuantizeOutput,
27-
ResidualQuantizerOutput,
28-
)
29-
from tzrec.modules.sid_generation.vector_quantize import (
30-
VectorQuantize,
31-
)
32-
33-
__all__ = [
34-
"QuantizeForwardMode",
35-
"QuantizeOutput",
36-
"ResidualQuantizerOutput",
37-
"VectorQuantize",
38-
"ResidualQuantizer",
39-
"ResidualVectorQuantizer",
40-
"KMeansLayer",
41-
"ResidualKMeansQuantizer",
42-
]

0 commit comments

Comments
 (0)