-
Notifications
You must be signed in to change notification settings - Fork 967
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
initialized buffer/gradInputs memory and added tests for dice #913
base: master
Are you sure you want to change the base?
Conversation
DICECriterion.lua
Outdated
return gradInput | ||
end | ||
|
||
function DICECriterion:accGradParameters(input, gradOutput) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed, as this is a Criterion. Criterions dont have accGradParameters
you need to add the criterion to init.lua , otherwise it wont be loaded. |
Is there any other thing I need to do apart from adding /home/lex/torch/install/share/lua/5.1/torch/Tester.lua:508: Couldn't find test 'DICECriterion' |
now the tests fail because your backward gradients dont match gradients from finite difference. there's likely a bug in your gradient calculation. |
That's because I added a normalizing constant to the gradient formula's denominator to avoid division errors. Maybe I should remove it?
|
can you make the normalizing constant be optional in the constructor, and disable it when running the tests... |
Sure Sincerely,
|
torch/install/share/lua/5.1/torch/Tester.lua:508: Couldn't find test 'DICECriterion' Was I supposed to do some tricks in Tester.lua as well? It seems to be taking longer than usual to make this PR work with Travis and your unit tests. Feel free to close/delete the PR. I will leave my implementation on my forked copy for at least a month from now if you or anyone can manage to integrate it into the unit tests. Cheers. |
Hi Lekan, |
It should merge. I believe I implemented it on a volumetric input back in the summer if I remember correctly. |
Hi
In this paper, it showed the gradient formula which I think is correct. |
Thanks for spotting this. I think I plugged the wrong equation into wolfram alpha to generate the gradients early on. Could you send a PR? |
I'm pretty busy right now. But I can send a PR maybe next January. |
This includes the memory initialization for buffer and gradInput. It also clarifies the definition of |X|.