Skip to content

Fix for derivative of sinc(x) when x is positive but very very small #56986

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

Closed
wants to merge 4 commits into from
Closed

Fix for derivative of sinc(x) when x is positive but very very small #56986

wants to merge 4 commits into from

Conversation

kbrose
Copy link
Contributor

@kbrose kbrose commented Apr 27, 2021

Problem arises for sinc'(x) where x != 0, but x ** 2 == 0, which happens for some very small floats.

I realized that my solution from #56763 was incomplete when I did a quick implementation using torch.autograd.Function and still got a NaN from my derivative.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 27, 2021

💊 CI failures summary and remediations

As of commit 0f4269a (more details on the Dr. CI page):


  • 2/2 failures possibly* introduced in this PR
    • 1/2 non-scanned failure(s)

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build binary_linux_libtorch_3_7m_cpu_gcc5_4_cxx11-abi_shared-with-deps_build (1/1)

Step: "Build" (full log | diagnosis details | 🔁 rerun)

FAILED: lib/libtorch_cpu.so
[5077/5411] Building CXX object test_cpp_rpc/CMakeFiles/test_cpp_rpc.dir/test_e2e_process_group.cpp.o
[5078/5411] Building CXX object test_api/CMakeFiles/test_api.dir/fft.cpp.o
[5079/5411] Building CXX object test_cpp_rpc/CMakeFiles/test_cpp_rpc.dir/test_e2e_tensorpipe.cpp.o
[5080/5411] Building CXX object test_api/CMakeFiles/test_api.dir/misc.cpp.o
[5081/5411] Building CXX object test_tensorexpr/CMakeFiles/test_tensorexpr.dir/test_memdependency.cpp.o
[5082/5411] Building CXX object test_api/CMakeFiles/test_api.dir/support.cpp.o
[5083/5411] Building CXX object dist_autograd/CMakeFiles/test_dist_autograd.dir/__/common/main.cpp.o
[5084/5411] Building CXX object test_api/CMakeFiles/test_api.dir/any.cpp.o
[5085/5411] Building CXX object test_api/CMakeFiles/test_api.dir/tensor_cuda.cpp.o
[5086/5411] Linking CXX shared library lib/libtorch_cpu.so
FAILED: lib/libtorch_cpu.so 
: && /usr/bin/c++ -fPIC -Wno-deprecated-declarations -D_GLIBCXX_USE_CXX11_ABI=1 -Wno-deprecated -fvisibility-inlines-hidden -DUSE_PTHREADPOOL -fopenmp -DUSE_KINETO -DUSE_FBGEMM -DUSE_QNNPACK -DUSE_PYTORCH_QNNPACK -DUSE_XNNPACK -O2 -fPIC -Wno-narrowing -Wall -Wextra -Werror=return-type -Wno-missing-field-initializers -Wno-type-limits -Wno-array-bounds -Wno-unknown-pragmas -Wno-sign-compare -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function -Wno-unused-result -Wno-unused-local-typedefs -Wno-strict-overflow -Wno-strict-aliasing -Wno-error=deprecated-declarations -Wno-psabi -Wno-error=pedantic -Wno-error=redundant-decls -Wno-error=old-style-cast -fdiagnostics-color=always -Wno-unused-but-set-variable -Wno-maybe-uninitialized -fno-math-errno -fno-trapping-math -Werror=format -DHAVE_AVX_CPU_DEFINITION -DHAVE_AVX2_CPU_DEFINITION -O2 -g -DNDEBUG  -Wl,--no-as-needed -rdynamic -shared -Wl,-soname,libtorch_cpu.so -o lib/libtorch_cpu.so caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/elementwise_sum_dnnlowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/fully_connected_fake_lowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/group_norm_dnnlowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/pool_dnnlowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/relu_dnnlowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/spatial_batch_norm_dnnlowp_op_avx2.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/transpose.cc.o caffe2/quantization/server/CMakeFiles/caffe2_dnnlowp_avx2_ops.dir/norm_minimization_avx2.cc.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/BatchedFallback.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/BatchedTensorImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/BatchingRegistrations.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/CPUGeneratorImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/Context.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/DLConvertor.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/DynamicLibrary.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ExpandUtils.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/LegacyTHFunctionsCPU.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/MemoryOverlap.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/NamedTensorUtils.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ParallelCommon.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ParallelNative.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ParallelNativeTBB.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ParallelOpenMP.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ParallelThreadPoolNative.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ScalarOps.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/SequenceNumber.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/SparseCsrTensorImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/SparseTensorImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/SparseTensorUtils.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorGeometry.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorIndexing.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorIterator.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorMeta.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorNames.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/TensorUtils.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/ThreadLocalState.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/Utils.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/Version.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/VmapMode.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/VmapModeRegistrations.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/VmapTransforms.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/autocast_mode.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/cpu/FlushDenormal.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/detail/CPUGuardImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/detail/CUDAHooksInterface.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/detail/HIPHooksInterface.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/detail/MetaGuardImpl.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/record_function.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/core/ATenGeneral.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/core/BackendSelectFallbackKernel.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/core/DeprecatedTypeProperties.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/core/DeprecatedTypePropertiesRegistry.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/aten/src/ATen/core/Dict.cpp.o caffe2/CMakeFiles/torch_cpu.dir/__/at
collect2: fatal error: ld terminated with signal 9 [Killed]
compilation terminated.
[5087/5411] Building CXX object test_api/CMakeFiles/test_api.dir/parameterlist.cpp.o
[5088/5411] Building CXX object test_api/CMakeFiles/test_api.dir/parameterdict.cpp.o
[5089/5411] Building CXX object caffe2/lib_c10d/CMakeFiles/c10d.dir/ParamCommsUtils.cpp.o
[5090/5411] Building CXX object test_api/CMakeFiles/test_api.dir/integration.cpp.o
[5091/5411] Building CXX object test_api/CMakeFiles/test_api.dir/moduledict.cpp.o
[5092/5411] Building CXX object test_api/CMakeFiles/test_api.dir/ordered_dict.cpp.o
[5093/5411] Building CXX object test_api/CMakeFiles/test_api.dir/modulelist.cpp.o

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@kbrose kbrose requested review from albanD and soulitzer as code owners April 27, 2021 03:54
@H-Huang H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Apr 27, 2021
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Thanks for the update.
That looks good!

@albanD
Copy link
Collaborator

albanD commented Apr 27, 2021

Could you rebase on top of master to properly trigger the CI please?

kbrose added 4 commits April 27, 2021 12:31
Problem arises for sinc'(x) where x > 0, but x ** 2 == 0
(which happens for some very small floats).
@kbrose
Copy link
Contributor Author

kbrose commented Apr 27, 2021

Rebased

@kbrose
Copy link
Contributor Author

kbrose commented Apr 28, 2021

@albanD seems like all the tests still didn't run. Let me know if I'm doing something wrong.

@albanD
Copy link
Collaborator

albanD commented Apr 28, 2021

Let me look into it.

@kbrose
Copy link
Contributor Author

kbrose commented Apr 29, 2021

I can also open a new PR from a clean branch if that's easier than debugging CI.

@albanD
Copy link
Collaborator

albanD commented Apr 29, 2021

No need, this is actually because we're getting rate limited by circle CI.

@albanD
Copy link
Collaborator

albanD commented Apr 29, 2021

Ok, opening another PR with the same commit did the trick :D Should be good now.

@kbrose
Copy link
Contributor Author

kbrose commented Apr 29, 2021

Apologies, I'm a little confused. Should I open a new MR?

@albanD
Copy link
Collaborator

albanD commented Apr 29, 2021

No need to do anything.
You should see the CI in this PR now.

Note that the libtorch fail seems unrelated.

@facebook-github-bot
Copy link
Contributor

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

@kbrose
Copy link
Contributor Author

kbrose commented Apr 29, 2021

Thanks for keeping this moving along @albanD, I appreciate it!

@facebook-github-bot
Copy link
Contributor

@albanD merged this pull request in ec86f96.

@kbrose kbrose deleted the sinc-derivative-numeric-fix branch April 29, 2021 22:21
krshrimali pushed a commit to krshrimali/pytorch that referenced this pull request May 19, 2021
…ytorch#56986)

Summary:
Problem arises for sinc'(x) where x != 0, but x ** 2 == 0, which happens for some very small floats.

I realized that my solution from pytorch#56763 was incomplete when I did a quick implementation using `torch.autograd.Function` and still got a `NaN` from my derivative.

Pull Request resolved: pytorch#56986

Reviewed By: gchanan

Differential Revision: D28093507

Pulled By: albanD

fbshipit-source-id: 2a30e1065b08c5c60de843a0778dedeb0fb295f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants