Quick fix for small issues with custom SUTs #457
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
| @@ -136,6 +136,7 @@ def content_sut(self, item: SutDescription, key: str): | |||
| if item.key in self._content: | |||
| return self._content[item.key][key] | |||
| else: | |||
| return f"{key} ({item})" | |||
There was a problem hiding this comment.
The raise below is unreachable now. Should we get rid of it?
There was a problem hiding this comment.
Ah, right. I specifically wanted to ask @dhosterman about this. For SUTs that we control, we'll put in friendly names. For others, I'm not sure how to handle the in-report name. This change keeps things from catching on fire, but it's not great otherwise. Any ideas?
There was a problem hiding this comment.
Since Daniel's out, I've change this to warn about the issue, as well as to do something reasonable when it's a name being looked for.
There was a problem hiding this comment.
Looks like a solid fix to me!
This may be a little out of scope, but this bug brought up a question for me: What's the intention in making ModelGaugeSut separate from SutDescription? I could understand this design choice if we had other types of SUTs besides ModelGaugeSut. But I think all SUTs will necessarily have to be a ModelGaugeSut.
|
What's the intention in making Good question. The other subclass of SutDescription is FakeSut, which is used when anonymizing reports. |
Hopefully fixes #453
Just a few small things that only make a difference when running a custom SUT. We should probably add a smoke test for this, but let's talk about what form that should take.