Skip to content

Adding Poisson NLL loss to libtorch #19316

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.

Sign up for GitHub

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

Jump to bottom
Closed
interesaaat wants to merge 13 commits into from
Closed

Adding Poisson NLL loss to libtorch #19316

interesaaat wants to merge 13 commits into from

Conversation

@interesaaat
Copy link
Contributor

@interesaaat interesaaat commented Apr 16, 2019

This PR add Poisson NLL loss to aten and substitute the python implementation with a call to the c++.

Fixes #19186.

ezyang
ezyang approved these changes Apr 19, 2019
View reviewed changes
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

facebook-github-bot
facebook-github-bot reviewed Apr 19, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezyang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ezyang
Copy link
Contributor

ezyang commented Apr 23, 2019

This doesn't seem to pass tests:

Apr 19 16:08:55 ======================================================================
Apr 19 16:08:55 ERROR: test_nn_poisson_nll_loss_full (__main__.TestJitGeneratedFunctional)
Apr 19 16:08:55 ----------------------------------------------------------------------
Apr 19 16:08:55 Traceback (most recent call last):
Apr 19 16:08:55   File "test_jit.py", line 13054, in wrapper
Apr 19 16:08:55     return fn(*args, **kwargs)
Apr 19 16:08:55   File "test_jit.py", line 13080, in do_test
Apr 19 16:08:55     output_variable = getattr(F, name)(self_variable, *args_variable, **kwargs_variable)
Apr 19 16:08:55   File "/opt/python/3.5/lib/python3.5/site-packages/torch/nn/functional.py", line 1931, in poisson_nll_loss
Apr 19 16:08:55     ret = torch.poisson_nll_loss(input, target, log_input, full, eps, _Reduction.get_enum(reduction))
Apr 19 16:08:55 IndexError: Can only index with tensors that are scalars (zero-dim)
Apr 19 16:08:55 
Apr 19 16:08:55 ----------------------------------------------------------------------

ezyang
ezyang requested changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs tests to pass. dismiss this review when ready

@interesaaat
Copy link
Contributor Author

interesaaat commented Apr 24, 2019

needs tests to pass. dismiss this review when ready

I am looking into this.

@interesaaat
Copy link
Contributor Author

interesaaat commented Apr 27, 2019

@ezyang I think the PR is ready now.

@ezyang
Copy link
Contributor

ezyang commented May 7, 2019

Still has Windows error:

1:31:35 ..\aten\src\ATen\native\Loss.cpp(140): error C2065: 'M_PI': undeclared identifier
01:31:35 ..\aten\src\ATen\native\Loss.cpp(140): error C2228: left of '.masked_select' must have class/struct/union
0

ezyang
ezyang approved these changes May 10, 2019
View reviewed changes
Copy link
Contributor

@ezyang ezyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really make some general _USE_MATH_DEFINES include for use

facebook-github-bot
facebook-github-bot reviewed May 10, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ezyang ezyang mentioned this pull request May 10, 2019
Open
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented May 10, 2019

@ezyang merged this pull request in 35fed93.

zdevito pushed a commit to zdevito/ATen that referenced this pull request May 10, 2019
@interesaaat @facebook-github-bot
Adding Poisson NLL loss to libtorch (#19316)
77b1756 
Summary:
This PR add Poisson NLL loss to aten and substitute the python implementation with a call to the c++.

Fixes #19186.
Pull Request resolved: pytorch/pytorch#19316

Differential Revision: D15012957

Pulled By: ezyang

fbshipit-source-id: 0a3f56e8307969c2f9cc321b5357a496c3d1784e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@ezyang ezyang ezyang approved these changes

Assignees

No one assigned

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Poisson NLL loss in libtorch

3 participants

@interesaaat @ezyang @facebook-github-bot