Metric versioning - Githubissues

GEM-benchmark / GEM-metrics

Automatic metrics for GEM tasks

https://gem-benchmark.com

MIT License

60 stars 20 forks source link

Metric versioning #93

Open jordiclive opened 2 years ago

jordiclive commented 2 years ago

A possible way to solve Issue #59.

We could make sure metric_version.py is run every new commit and then each metric can have an attribute loaded from the resulting json or yaml file.

metric_version.py finds the last commit that modified the respective metric file.

tuetschek commented 2 years ago

@jordiclive thanks, this looks like a good idea! I'd have a few questions & proposals:

Could we perhaps keep this right under gem_metrics/? The impl/ subdirectory only has some additional code for some of the metrics.
It would be good if the location of the json file was given by os.path.join(os.path.abspath(os.path.dirname('__file__')), 'metric_version.json') (or something more elegant along the same lines), so no matter where you call the script from, it always saves into the same path.
Maybe we also want to have a function that returns the version of a metric? Or maybe even a method in AbstractMetric?
Maybe we should put a list of all available metrics in one place somewhere into gem_metrics/__init__.py? It's actually kinda weird that we don't have it yet, and it might be useful. There's the metric_name_to_metric_class dict, so maybe base it on that?
Do you think there's a way of automating this somehow?

jordiclive commented 2 years ago

@tuetschek. Thanks for taking a look :) Yep, definitely agree with all those!

With automating, if you meanmetric_version.py to run every new commit, I imagine a git hook?

tuetschek commented 2 years ago

@jordiclive Yeah that's what I meant – but I'm actually not sure how to do it, since you first need to commit the metric and then run the script, which would essentially need another commit to store the version hashes 🤔?

jordiclive commented 2 years ago

@tuetschek, yes you are right. And would be getting quite complicated! This is probably something for setuptools or having json saved somewhere else.

tuetschek commented 2 years ago

@jordiclive Does this mean you think it's too complicated now 🙂? I guess we could just have some tests that would essentially force you to commit the JSON every time you're changing a metric file... but yeah, maybe it would take time to figure out.

jordiclive commented 2 years ago

Ha, no I'm happy to continue! I just thought there may be a more elegant way with setuptools.

Maybe if a commit is logged before the force commit. As we need two commits and then it to stop. I think it is possible with a post-commit hook. I will look into that.