dataclass_compat: track initial tag metadata

[wchargin]

Summary:
The dataclass_compat transform dispatches on plugin name, which is
stored in the summary metadata. But the semantics of summary metadata is
that it need only be set in the first event of each time series. Thus,
until now, events past the first step of a time series have not been
modified by dataclass_compat. This has been fine because all
transformations have been either (a) only metadata transforms (which
don’t matter after the first step anyway) or (b) tensor transforms to
hparams data (which only ever has one step anyway). But an upcoming
change will need to transform every step of each audio time series, so
we need to identify which summaries have audio data even when they don’t
have plugin name set on the same event.

To do so, we let dataclass_compat update a stateful map from tag name
to the first SummaryMetadata seen for that time series (scoped to a
particular run).

Test Plan:
Some unit tests included now; the tests for migration of audio summaries
will be more complete because they have more interesting things to test.

wchargin-branch: dataclass-compat-initial-metadata

[nfelt]

Thanks for documenting this!

[wchargin]

Yep. Hopefully no one ever has to read it.

[nfelt]

Just to check my understanding, our assumption is that any summary that natively sets the data class on the initial metadata will already be using the dataclass-aware format (so to later summaries with that tag will require value conversions)?

[wchargin]

Yeah. In some sense there’s a bit of awkward coupling: one could imagine
factoring the migrations more finely as

1. data_compat (v1 → v2 summaries)
2. audio summary projection (label removal)
3. hparams tensor fix
4. dataclass_compat (add metadata.data_class only)

where there are some dependencies (4 depends on 2, 2 depends on 1) but
the dependency graph is not (an orientation of) a complete graph. Then
if we needed to add more read-time migrations later even after summaries
natively write data_class we’d still be able to do so.

For now, at least, I thought that it was simpler to keep everything in
this layer so that there are fewer moving pieces.

[nfelt]

FWIW, I guess this behavior is fine, but it would also seem fine to me to populate the dataclass in the metadata if the metadata is present on non-initial steps (i.e. base it just on whether that summary has metadata or not) which seems a little less stateful and thus perhaps slightly simpler to reason about?

[wchargin]

Okay, that’s fine with me. It’s important that we not populate the
metadata where it didn’t previously exist, because (e.g.) this triggers
a “mismatching plugin names” ("" != "scalars") warning in the
uploader. But if the metadata already exists I don’t see any harm in
populating the data class field. Done, and removed initial args.