-
Notifications
You must be signed in to change notification settings - Fork 2
Adding basic MetricSignature functionality #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding basic MetricSignature functionality #16
Conversation
pose_evaluation/metrics/base.py
Outdated
|
|
||
| self.signature_info = { | ||
| "name": name, | ||
| "higher_is_better": args.get("higher_is_better", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i suggest extending it via
self.signature_info = {"name": name, **args}that way, it is easily extendable. also, higher_is_better never has None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can have None if it is not a Metric signature, but a Signature of, say, a DistanceMeasure, or an Aggregator or so forth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or that is to say, some Signatures might not have the "higher is better" field at all, once we extend the use past Metrics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The {**args} idea does seem good! Tried implementing it that way, and realized that if the subclass defines something like kind, but doesn't define an abbreviation, we get a crash. Hmmm
|
Pushed a new version. This is what the full and short signature looks like when you do BaseMetric("base") and DistanceMetric() Note how DistanceMetric defines |
Breaking this out of #4. Following the conventions of sacrebleu, implementing Signature class.