metrics: gracefully degrade without tensors/blobs #4425
Conversation
Summary: Not all data providers support tensors and blob sequences. The methods are optional in the `DataProvider` interface, and (e.g.) the gRPC data provider does not yet implement them, nor does the RustBoard server. This patch teaches the time series dashboard to gracefully omit image and histogram data if the needed data classes aren’t available. Test Plan: Point TensorBoard at an MNIST dataset with all three kinds of data, and note that scalars appear if `--load_fast` is passed, and all data appears otherwise. Prior to this patch, the requests would 500 under `--load_fast`, and so nothing would be displayed at all. (Cherry-pick #4411 for `--load_fast`.) wchargin-branch: metrics-degrade-tensors-blobs wchargin-source: 0b486e4639b83c309b626770ae46527ec0b5460b
| if histogram_mapping is None: | ||
| histogram_mapping = {} | ||
| if image_mapping is None: | ||
| image_mapping = {} |
There was a problem hiding this comment.
If unimplemented DataProvider methods may return None, can we add the corresponding if scalar_mapping is None check as well, so that metrics_plugin.py doesn't rely on knowing which specific dataclasses have implemented list_*?
On the other hand, should GrpcDataProvider be responsible for implementing list_tensors that returns {}? [1] suggests that instances of classes with @abc.abstractmethod must have "all of its abstract methods and properties are overridden". Would it make sense to add GrpcDataProvider.list_tensors/blobs, or update the data/provider.py docstrings to indicate that they may return None?
[1] https://docs.python.org/3/library/abc.html#abc.abstractmethod
There was a problem hiding this comment.
The scalars methods are abc.abstractmethods whereas the methods for
other data classes are not. You cannot instantiate a DataProvider
without implementing read_scalars. It’ll hard-error.
That these methods pass when not implemented is (in my experience)
fairly standard. For example, the abc docs do so:
class C(ABC):
@abstractmethod
def my_abstract_method(self):
... # will return None; this is equivalent to `pass`But I’m happy to document it explicitly. Done.
There was a problem hiding this comment.
The scalars methods are abc.abstractmethods whereas the methods for
other data classes are not
Ahh, thanks for pointing this out. I think my comment earlier was invalid, so feel free to keep or remove the data/provider.py change as you see fit!
wchargin-branch: metrics-degrade-tensors-blobs wchargin-source: 1484a0927d7c357c0a3ebf09eaed36ff38f05394
2 participants
Summary:
Not all data providers support tensors and blob sequences. The methods
are optional in the
DataProviderinterface, and (e.g.) the gRPC dataprovider does not yet implement them, nor does the RustBoard server.
This patch teaches the time series dashboard to gracefully omit image
and histogram data if the needed data classes aren’t available.
Test Plan:
Point TensorBoard at an MNIST dataset with all three kinds of data, and
note that scalars appear if
--load_fastis passed, and all dataappears otherwise. Prior to this patch, the requests would 500 under
--load_fast, and so nothing would be displayed at all.wchargin-branch: metrics-degrade-tensors-blobs