benchmark icon indicating copy to clipboard operation
benchmark copied to clipboard

Updated __init__.py with domain and task class attributes

Open nikithamalgifb opened this issue 4 years ago • 5 comments

Fixes issue: #209 All the models have been updated with domain and class attributes. This change is in correlation with the task of eliminating score.yml from the torchbenchmark framework.

nikithamalgifb avatar Jan 28 '21 13:01 nikithamalgifb

What do you think about making an enum class in one place where all the domains and tasks are listed, and then using the enum values here rather than just using strings?

with the string approach, i'm just worried about new models being added and people not knowing what the existing categories are, maybe making new ones up when using existing ones would have been better...

I think it's fine if people add new categories later, (it would impact the score but it would be for a new score version anyway) but it'd be good to have some friction to adding new ones without good reason. WDYT?

This makes total sense, let me change.

nikithamalgifb avatar Jan 28 '21 19:01 nikithamalgifb

also, i only counted 20 models with labels added, but we have 25 models. I'm not sure if all 5 of the missing models are ones we are disabling for now, but even if they are disabled shouldn't we label them and use another (more explicit) way to exclude them from the score than just leaving them unlabled?

wconstab avatar Feb 04 '21 17:02 wconstab

I would rather not introduce an enum value for 'NOT_DEFINED' since it's essentially just a short-term hack.

If you prefer to land this PR first and then implement the part that separates an explicit list of models and adds the missing categories later, thats OK (but I'd want you to commit to following up on that soon); but I would also be happy to help with labeling the tasks on the missing 5 now.

SuperSloMo: CV--OtherCV Tacotron: Speech--Synthesis which other ones are disabled?

wconstab avatar Feb 04 '21 21:02 wconstab

I would rather not introduce an enum value for 'NOT_DEFINED' since it's essentially just a short-term hack.

If you prefer to land this PR first and then implement the part that separates an explicit list of models and adds the missing categories later, thats OK (but I'd want you to commit to following up on that soon); but I would also be happy to help with labeling the tasks on the missing 5 now.

SuperSloMo: CV--OtherCV Tacotron: Speech--Synthesis which other ones are disabled?

Models currently not labelled:

  • maml - Domain: Reinforcement Learning? Task : Classification?
  • vgg16 - Domain: Computer Vision Task: classification/Detection?
  • squeezenet - Domain: Computer Vision Task: Classification?

Can you confirm if these are right? I'll just add them in this PR

nikithamalgifb avatar Feb 04 '21 23:02 nikithamalgifb

Hi @nikithamalgifb!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Sep 10 '22 07:09 facebook-github-bot