Skip to content

SCID trainer silently drops the last sample on odd mini-batches in _mmd_sets #19

Description

@Gouhuan1999
# 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions