# anoship-scid/scid_ad/training.py:56
def _mmd_sets(self, z_fact: torch.Tensor, z_cf: torch.Tensor):
B = z_fact.shape[0]
if B >= 4:
half = B // 2
return z_fact[:half], z_fact[half : 2 * half], z_cf[:half]
return z_fact, z_fact, z_cf
The docstring on this method describes splitting each mini-batch in half to form
the two sets the MMD term compares. For an odd batch size the split quietly
discards one row: with B = 5, half = 2, so the second slice is
z_fact[2:4] — index 4 is never used. With B = 7 two rows are dropped, etc.
It's not a crash, just a silent data loss of up to one sample per batch. The
reason it hasn't been noticed is that the tests only exercise even batch sizes
(the SCID training tests use batch sizes like 16/24), so the odd-B path is
never hit.
If the intent is "use as many paired samples as possible", the second slice
should run to the end of the matched region, e.g. z_fact[half:2*half] is fine
but the first set should also be 2*half long — or simpler, compute
half = B // 2 and consistently use [:half] and [half:2*half] while
acknowledging the dropped tail, or pad to an even count. Either way it's worth a
comment or a fix, because right now the behavior (drop the tail) is implicit and
undocumented.
Minor severity, but easy to get right.
The docstring on this method describes splitting each mini-batch in half to form
the two sets the MMD term compares. For an odd batch size the split quietly
discards one row: with
B = 5,half = 2, so the second slice isz_fact[2:4]— index4is never used. WithB = 7two rows are dropped, etc.It's not a crash, just a silent data loss of up to one sample per batch. The
reason it hasn't been noticed is that the tests only exercise even batch sizes
(the SCID training tests use batch sizes like 16/24), so the odd-
Bpath isnever hit.
If the intent is "use as many paired samples as possible", the second slice
should run to the end of the matched region, e.g.
z_fact[half:2*half]is finebut the first set should also be
2*halflong — or simpler, computehalf = B // 2and consistently use[:half]and[half:2*half]whileacknowledging the dropped tail, or pad to an even count. Either way it's worth a
comment or a fix, because right now the behavior (drop the tail) is implicit and
undocumented.
Minor severity, but easy to get right.