Bug description
RetrievalMRR.forward() can discard state accumulated before the call if
computing the current batch raises.
The example below first stores a valid query with index=0. The following
forward() call raises the expected empty-target error for a new query with
index=1. After the exception, query 0 is no longer present in the metric state.
Minimal reproduction
import torch
from torchmetrics.retrieval import RetrievalMRR
metric = RetrievalMRR(empty_target_action="error", compute_with_cache=False)
metric.update(torch.tensor([0.9]), torch.tensor([1]), indexes=torch.tensor([0]))
try:
metric(torch.tensor([0.8]), torch.tensor([0]), indexes=torch.tensor([1]))
except ValueError as error:
print("expected error:", error)
else:
raise AssertionError("Expected the configured empty-target error")
print("stored query IDs after failure:", torch.cat(metric.indexes).tolist())
metric.update(torch.tensor([0.7]), torch.tensor([1]), indexes=torch.tensor([1]))
print("observed MRR:", metric.compute().item())
def reference(preds, targets, indexes):
fresh = RetrievalMRR(empty_target_action="error", compute_with_cache=False)
fresh.update(
torch.tensor(preds),
torch.tensor(targets),
indexes=torch.tensor(indexes),
)
return fresh.compute().item()
print(
"retain failed batch:",
reference([0.9, 0.8, 0.7], [1, 0, 1], [0, 1, 1]),
)
print(
"discard failed batch:",
reference([0.9, 0.7], [1, 1], [0, 1]),
)
Actual behavior
stored query IDs after failure: [1]
observed MRR: 0.5
retain failed batch: 0.75
discard failed batch: 1.0
Query 0 was accumulated before the failing forward() call but is gone
afterwards. The reported 0.5 only evaluates query 1.
Expected behavior
I am not sure whether a failed forward() call is intended to keep or roll back
the new batch. Either behavior could be reasonable, but state accumulated before
the call should remain intact.
- If the failed batch is kept, the final MRR is
0.75.
- If the failed batch is rolled back, the final MRR is
1.0.
- The current result is
0.5 because the earlier query is lost.
The base forward paths restore or merge the previous state only after the batch
computation returns. If that computation raises, the restoration step is skipped.
This appears separate from #3486, which covers cleanup inside sync_context.
What exception behavior should forward() guarantee? Once that is clear, I can
add a regression test and patch.
Environment
- Python 3.13.5
- PyTorch 2.10.0+cpu
- TorchMetrics 1.9.0
- Linux
- Single process
The same save/reset/compute/restore ordering is present on master at
8d008de1660b18ba44fb1596f8a5e9e8361ba55c. I have not run the full test suite
from that checkout.
Related implementation discussions: #344 and #984.
Bug description
RetrievalMRR.forward()can discard state accumulated before the call ifcomputing the current batch raises.
The example below first stores a valid query with
index=0. The followingforward()call raises the expected empty-target error for a new query withindex=1. After the exception, query 0 is no longer present in the metric state.Minimal reproduction
Actual behavior
Query 0 was accumulated before the failing
forward()call but is goneafterwards. The reported
0.5only evaluates query 1.Expected behavior
I am not sure whether a failed
forward()call is intended to keep or roll backthe new batch. Either behavior could be reasonable, but state accumulated before
the call should remain intact.
0.75.1.0.0.5because the earlier query is lost.The base forward paths restore or merge the previous state only after the batch
computation returns. If that computation raises, the restoration step is skipped.
This appears separate from #3486, which covers cleanup inside
sync_context.What exception behavior should
forward()guarantee? Once that is clear, I canadd a regression test and patch.
Environment
The same save/reset/compute/restore ordering is present on
masterat8d008de1660b18ba44fb1596f8a5e9e8361ba55c. I have not run the full test suitefrom that checkout.
Related implementation discussions: #344 and #984.