Feature/is nan #7068
Feature/is nan #7068
Conversation
Make them as usual names.
Make them as usual names.
Make them as usual names.
…dle into feature/enhance_dev_ctx_pool
paddle/framework/tensor_util.cc
Outdated
| void operator()() const { | ||
| auto t = EigenVector<T>::Flatten(tensor_); | ||
| auto o = EigenScalar<bool>::From(*out_); | ||
| o.device(*ctx_.eigen_device()) = predicate_(t).any(); |
There was a problem hiding this comment.
We can add a reference comment on eigen any() for better understanding
https://eigen.tuxfamily.org/dox/classEigen_1_1DenseBase.html#abfbf4cb72dd577e62fbe035b1c53e695
paddle/framework/tensor_util.cc
Outdated
| template <typename T> | ||
| auto operator()(const T& eigen_vec) const | ||
| -> decltype(std::declval<T>().isnan()) { | ||
| return eigen_vec.isnan(); |
There was a problem hiding this comment.
add a reference comment:
https://eigen.tuxfamily.org/dox/classEigen_1_1ArrayBase.html#aab10b156cb69206461728782dd01c37a
paddle/framework/tensor_util.cc
Outdated
| tmp.Resize({1}); | ||
| tmp.mutable_data<bool>(cpu); | ||
| platform::DeviceContextPool::Instance().Get(gpu)->Wait(); | ||
| CopyFrom(out, cpu, &tmp); |
There was a problem hiding this comment.
If we do copy between CPU and GPU, we need to pass a DeviceContext
There was a problem hiding this comment.
However, the device context is a global variable. We can get DeviceContext by its place.
There was a problem hiding this comment.
I mean the CopyFrom interface is
Paddle/paddle/framework/tensor_util.h
Lines 33 to 34 in 1831176
| @@ -230,5 +231,28 @@ TEST(CopyToVector, Tensor) { | |||
| #endif | |||
| } | |||
|
|
|||
| TEST(IsNAN, CPU) { | |||
There was a problem hiding this comment.
Please add GPU unit tests at the same time.
| @@ -52,6 +52,14 @@ class CPUDeviceContext : public DeviceContext { | |||
| std::unique_ptr<Eigen::DefaultDevice> eigen_device_; | |||
| }; | |||
|
|
|||
| template <typename Place> | |||
| struct DefaultDeviceContextType; | |||
There was a problem hiding this comment.
What is the DefaultDeviceContextType used for?
There was a problem hiding this comment.
It is used for DeviceCtxPool::GetByPlace() method. This method will return the casted DeviceContext by PlaceType.
In the future, we could add a library type to this method.
| @@ -207,6 +209,12 @@ inline void CopyToVector(const Tensor& src, std::vector<T>* dst) { | |||
| src_ptr, size); | |||
| } | |||
|
|
|||
| // Returns true if a tensor contains NAN, i.e., Not A Number. | |||
| extern bool HasNAN(const framework::Tensor& tensor); | |||
| @@ -0,0 +1 @@ | |||
| ./tensor_util.cc | |||
There was a problem hiding this comment.
Why not move the code in tensor_util.cc to tensor_util.h. Or we can have a tensor_util_impl.h. It's a little strange to have a symbolic link.
There was a problem hiding this comment.
@QiJune Reyoung probably had an offline discussion with you already. But looks like .cu is necessary to make nvcc pass the compilation...
| ASSERT_TRUE(HasNAN(src)); | ||
| } | ||
|
|
||
| TEST(IsInf, CPU) { |
This is a part of #7092 .