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
Proposal in "model/loss.py" - Use loss classes instead of functional API #21
Comments
Reference (example): To clarify:
* F.nll_loss(y_input, y_target) ** F.nll_loss(y_input, y_target, weight=self.weight, ignore_index=self.ignore_index, reduction=self.reduction)``` |
Hi, with your examples, I do agree that using loss classes is better than using functional APIs. Though the purpose of this part in the template is just an example, I do think using loss objects in the example code could provide a better convention for someone who is not familiar with pytorch. |
Hi again, I have already implemented this in an offline copy of the repository. Should I create a pull request with the proposal? The pull request will then update:
What I likely not implement for this pull request (todo for later):
|
Hi, since this is a relatively minor change, I think combining this and the future custom loss feature in a single PR should be fine, thanks |
I have created a commit with updates addressing this issue. While I agree that my last bullet is a small change, however, I am not certain how to best implement it. Therefore I left it as an 'TODO', (which I also mention that in the updated README.md). PS: I need to fix "master" on my fork to synchronize with this repository. If you merge pull request #15, then I will pretty much be back on track. Thereafter I will create a tidy pull request for this issue. Sorry for that inconvenience and thanks. |
Hi,
Currently, 'get_loss_function' queries for local functions in the same file and returns the object, if it finds a function with the matching name. I propose that we instead use the interface of 'torch.nn.modules.loss' classes and return an instantiated class object, instead of a function reference.
These classes either way call the functional API, and are better documented.
As an example:
What do you think?
The text was updated successfully, but these errors were encountered: