Skip to content
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

Design debate: Should we use hydra's object instantiation feature? #22

Open
vinamarora8 opened this issue Nov 12, 2024 · 1 comment
Open
Labels
enhancement New feature or request

Comments

@vinamarora8
Copy link
Member

vinamarora8 commented Nov 12, 2024

In my opinion, good research release code is simple, easy to read, and transparent. Instantiating models and tokenizers using hydra.utils.instantiate(), however, is very opaque and requires readers to understand hydra's configuration loading system.
This discussion is only regarding examples/poyo/train.py. I like to use this hydra feature when developing code and doing experiments too. But I don't see it having a place in released research code.

@mazabou mazabou added the enhancement New feature or request label Nov 13, 2024
@mazabou
Copy link
Member

mazabou commented Nov 13, 2024

Currently hydra.utils.instantiate() is used for the model, the transforms, and the metrics.

  1. For the model, I agree we can call POYOPlus directly. The only advantage of having the class in the config file is that it makes it super clear what the destination for the model is. which is actually pretty neat. if we remove it, it might not be as explicit. of course we can always add a comment.

  2. For the transforms, composing augmentations is pretty neat imo. what would be the alternative here?

  3. For the metrics, they are hidden, so should we care about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants