From 61052cdbc6cd048410aebb0df514fba6f8931347 Mon Sep 17 00:00:00 2001 From: chenweihang Date: Wed, 8 Aug 2018 10:22:36 +0000 Subject: [PATCH 1/3] polish high frequency enforce error message --- paddle/fluid/platform/enforce.h | 10 ++++++---- paddle/fluid/platform/gpu_info.cc | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/paddle/fluid/platform/enforce.h b/paddle/fluid/platform/enforce.h index 566485cd3c..cad60275a2 100644 --- a/paddle/fluid/platform/enforce.h +++ b/paddle/fluid/platform/enforce.h @@ -263,7 +263,8 @@ inline void throw_on_error(T e) { * PADDLE_ENFORCE_EQ(a, b); * * will raise an expression described as follows: - * "enforce a == b failed, 1 != 2" with detailed stack information. + * "Data check failed. Expected input a == b, but received a(1) != b(2)." + * with detailed stack information. * * extra messages is also supported, for example: * PADDLE_ENFORCE(a, b, "some simple enforce failed between %d numbers", 2) @@ -292,9 +293,10 @@ inline void throw_on_error(T e) { #define __PADDLE_BINARY_COMPARE(__VAL0, __VAL1, __CMP, __INV_CMP, ...) \ do { \ if (UNLIKELY(!((__VAL0)__CMP(__VAL1)))) { \ - PADDLE_THROW("enforce %s " #__CMP " %s failed, %s " #__INV_CMP \ - " %s\n%s", \ - #__VAL0, #__VAL1, paddle::string::to_string(__VAL0), \ + PADDLE_THROW("Data check failed. Expected %s " #__CMP \ + " %s, but received %s:%s " #__INV_CMP " %s:%s.\n%s", \ + #__VAL0, #__VAL1, #__VAL0, \ + paddle::string::to_string(__VAL0), #__VAL1, \ paddle::string::to_string(__VAL1), \ paddle::string::Sprintf("" __VA_ARGS__)); \ } \ diff --git a/paddle/fluid/platform/gpu_info.cc b/paddle/fluid/platform/gpu_info.cc index 4cee93f3a4..f9e2e8c69d 100644 --- a/paddle/fluid/platform/gpu_info.cc +++ b/paddle/fluid/platform/gpu_info.cc @@ -100,25 +100,25 @@ size_t GpuMinChunkSize() { size_t GpuMaxChunkSize() { size_t total = 0; - size_t available = 0; + size_t available_memory = 0; - GpuMemoryUsage(&available, &total); - VLOG(10) << "GPU Usage " << available / 1024 / 1024 << "M/" + GpuMemoryUsage(&available_memory, &total); + VLOG(10) << "GPU Usage " << available_memory / 1024 / 1024 << "M/" << total / 1024 / 1024 << "M"; size_t reserving = static_cast(0.05 * total); // If available less than minimum chunk size, no usable memory exists. - available = - std::min(std::max(available, GpuMinChunkSize()) - GpuMinChunkSize(), - total - reserving); + available_memory = std::min( + std::max(available_memory, GpuMinChunkSize()) - GpuMinChunkSize(), + total - reserving); // Reserving the rest memory for page tables, etc. - size_t allocating = static_cast(FLAGS_fraction_of_gpu_memory_to_use * - (total - reserving)); + size_t allocating_memory = static_cast( + FLAGS_fraction_of_gpu_memory_to_use * (total - reserving)); - PADDLE_ENFORCE_LE(allocating, available); + PADDLE_ENFORCE_LE(allocating_memory, available_memory); - return allocating; + return allocating_memory; } void GpuMemcpyAsync(void *dst, const void *src, size_t count, From b1dd4149b90dde40640de2baf0190d611cb24486 Mon Sep 17 00:00:00 2001 From: chenweihang Date: Thu, 9 Aug 2018 03:02:25 +0000 Subject: [PATCH 2/3] adjust enforce test cases --- paddle/fluid/platform/enforce_test.cc | 30 +++++++++++++++++---------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/paddle/fluid/platform/enforce_test.cc b/paddle/fluid/platform/enforce_test.cc index 0e8684581a..8dcf39fdaa 100644 --- a/paddle/fluid/platform/enforce_test.cc +++ b/paddle/fluid/platform/enforce_test.cc @@ -54,7 +54,9 @@ TEST(ENFORCE_EQ, NO_EXTRA_MSG_FAIL) { PADDLE_ENFORCE_EQ(a, 1 + 3); } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; - HasPrefix(StringPiece(error.what()), "enforce a == 1 + 3 failed, 2 != 4"); + HasPrefix( + StringPiece(error.what()), + "Data check failed. Expected a == 1 + 3, but received a:2 != 1 + 3:4."); } EXPECT_TRUE(caught_exception); } @@ -67,7 +69,8 @@ TEST(ENFORCE_EQ, EXTRA_MSG_FAIL) { } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; HasPrefix(StringPiece(error.what()), - "enforce a == 1 + 3 failed, 2 != 4\ntheir size not match"); + "Data check failed. Expected a == 1 + 3, but received a:2 != 1 + " + "3:4.\ntheir size not match"); } EXPECT_TRUE(caught_exception); } @@ -84,8 +87,9 @@ TEST(ENFORCE_NE, FAIL) { PADDLE_ENFORCE_NE(1.0, 1UL); } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; - EXPECT_TRUE(HasPrefix(StringPiece(error.what()), - "enforce 1.0 != 1UL failed, 1 == 1")) + EXPECT_TRUE(HasPrefix( + StringPiece(error.what()), + "Data check failed. Expected 1.0 != 1UL, but received 1.0:1 == 1UL:1.")) << error.what() << " does not have expected prefix"; } EXPECT_TRUE(caught_exception); @@ -98,8 +102,9 @@ TEST(ENFORCE_GT, FAIL) { PADDLE_ENFORCE_GT(1, 2UL); } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; - EXPECT_TRUE( - HasPrefix(StringPiece(error.what()), "enforce 1 > 2UL failed, 1 <= 2")); + EXPECT_TRUE(HasPrefix( + StringPiece(error.what()), + "Data check failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -116,8 +121,9 @@ TEST(ENFORCE_GE, FAIL) { PADDLE_ENFORCE_GE(1, 2UL); } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; - EXPECT_TRUE( - HasPrefix(StringPiece(error.what()), "enforce 1 >= 2UL failed, 1 < 2")); + EXPECT_TRUE(HasPrefix( + StringPiece(error.what()), + "Data check failed. Expected 1 >= 2UL, but received 1:1 < 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -135,8 +141,9 @@ TEST(ENFORCE_LE, FAIL) { PADDLE_ENFORCE_GT(1, 2UL); } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; - EXPECT_TRUE( - HasPrefix(StringPiece(error.what()), "enforce 1 > 2UL failed, 1 <= 2")); + EXPECT_TRUE(HasPrefix( + StringPiece(error.what()), + "Data check failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -153,7 +160,8 @@ TEST(ENFORCE_LT, FAIL) { } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; EXPECT_TRUE(HasPrefix(StringPiece(error.what()), - "enforce 1UL < 0.12 failed, 1 >= 0.12")); + "Data check failed. Expected 1UL < 0.12, but " + "received 1UL:1 >= 0.12:0.12.")); } EXPECT_TRUE(caught_exception); } From da39d84a48d1445d6bb9fb10e8d7d17d9053c7b7 Mon Sep 17 00:00:00 2001 From: chenweihang Date: Tue, 14 Aug 2018 09:55:47 +0000 Subject: [PATCH 3/3] refine by reviewer's advice --- paddle/fluid/platform/enforce.h | 4 ++-- paddle/fluid/platform/enforce_test.cc | 14 +++++++------- paddle/fluid/platform/gpu_info.cc | 21 +++++++++++---------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/paddle/fluid/platform/enforce.h b/paddle/fluid/platform/enforce.h index cad60275a2..81b5359b40 100644 --- a/paddle/fluid/platform/enforce.h +++ b/paddle/fluid/platform/enforce.h @@ -263,7 +263,7 @@ inline void throw_on_error(T e) { * PADDLE_ENFORCE_EQ(a, b); * * will raise an expression described as follows: - * "Data check failed. Expected input a == b, but received a(1) != b(2)." + * "Enforce failed. Expected input a == b, but received a(1) != b(2)." * with detailed stack information. * * extra messages is also supported, for example: @@ -293,7 +293,7 @@ inline void throw_on_error(T e) { #define __PADDLE_BINARY_COMPARE(__VAL0, __VAL1, __CMP, __INV_CMP, ...) \ do { \ if (UNLIKELY(!((__VAL0)__CMP(__VAL1)))) { \ - PADDLE_THROW("Data check failed. Expected %s " #__CMP \ + PADDLE_THROW("Enforce failed. Expected %s " #__CMP \ " %s, but received %s:%s " #__INV_CMP " %s:%s.\n%s", \ #__VAL0, #__VAL1, #__VAL0, \ paddle::string::to_string(__VAL0), #__VAL1, \ diff --git a/paddle/fluid/platform/enforce_test.cc b/paddle/fluid/platform/enforce_test.cc index 8dcf39fdaa..d521829655 100644 --- a/paddle/fluid/platform/enforce_test.cc +++ b/paddle/fluid/platform/enforce_test.cc @@ -56,7 +56,7 @@ TEST(ENFORCE_EQ, NO_EXTRA_MSG_FAIL) { caught_exception = true; HasPrefix( StringPiece(error.what()), - "Data check failed. Expected a == 1 + 3, but received a:2 != 1 + 3:4."); + "Enforce failed. Expected a == 1 + 3, but received a:2 != 1 + 3:4."); } EXPECT_TRUE(caught_exception); } @@ -69,7 +69,7 @@ TEST(ENFORCE_EQ, EXTRA_MSG_FAIL) { } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; HasPrefix(StringPiece(error.what()), - "Data check failed. Expected a == 1 + 3, but received a:2 != 1 + " + "Enforce failed. Expected a == 1 + 3, but received a:2 != 1 + " "3:4.\ntheir size not match"); } EXPECT_TRUE(caught_exception); @@ -89,7 +89,7 @@ TEST(ENFORCE_NE, FAIL) { caught_exception = true; EXPECT_TRUE(HasPrefix( StringPiece(error.what()), - "Data check failed. Expected 1.0 != 1UL, but received 1.0:1 == 1UL:1.")) + "Enforce failed. Expected 1.0 != 1UL, but received 1.0:1 == 1UL:1.")) << error.what() << " does not have expected prefix"; } EXPECT_TRUE(caught_exception); @@ -104,7 +104,7 @@ TEST(ENFORCE_GT, FAIL) { caught_exception = true; EXPECT_TRUE(HasPrefix( StringPiece(error.what()), - "Data check failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); + "Enforce failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -123,7 +123,7 @@ TEST(ENFORCE_GE, FAIL) { caught_exception = true; EXPECT_TRUE(HasPrefix( StringPiece(error.what()), - "Data check failed. Expected 1 >= 2UL, but received 1:1 < 2UL:2.")); + "Enforce failed. Expected 1 >= 2UL, but received 1:1 < 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -143,7 +143,7 @@ TEST(ENFORCE_LE, FAIL) { caught_exception = true; EXPECT_TRUE(HasPrefix( StringPiece(error.what()), - "Data check failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); + "Enforce failed. Expected 1 > 2UL, but received 1:1 <= 2UL:2.")); } EXPECT_TRUE(caught_exception); } @@ -160,7 +160,7 @@ TEST(ENFORCE_LT, FAIL) { } catch (paddle::platform::EnforceNotMet error) { caught_exception = true; EXPECT_TRUE(HasPrefix(StringPiece(error.what()), - "Data check failed. Expected 1UL < 0.12, but " + "Enforce failed. Expected 1UL < 0.12, but " "received 1UL:1 >= 0.12:0.12.")); } EXPECT_TRUE(caught_exception); diff --git a/paddle/fluid/platform/gpu_info.cc b/paddle/fluid/platform/gpu_info.cc index f9e2e8c69d..126636d879 100644 --- a/paddle/fluid/platform/gpu_info.cc +++ b/paddle/fluid/platform/gpu_info.cc @@ -100,25 +100,26 @@ size_t GpuMinChunkSize() { size_t GpuMaxChunkSize() { size_t total = 0; - size_t available_memory = 0; + size_t available = 0; - GpuMemoryUsage(&available_memory, &total); - VLOG(10) << "GPU Usage " << available_memory / 1024 / 1024 << "M/" + GpuMemoryUsage(&available, &total); + VLOG(10) << "GPU Usage " << available / 1024 / 1024 << "M/" << total / 1024 / 1024 << "M"; size_t reserving = static_cast(0.05 * total); // If available less than minimum chunk size, no usable memory exists. - available_memory = std::min( - std::max(available_memory, GpuMinChunkSize()) - GpuMinChunkSize(), - total - reserving); + available = + std::min(std::max(available, GpuMinChunkSize()) - GpuMinChunkSize(), + total - reserving); // Reserving the rest memory for page tables, etc. - size_t allocating_memory = static_cast( - FLAGS_fraction_of_gpu_memory_to_use * (total - reserving)); + size_t allocating = static_cast(FLAGS_fraction_of_gpu_memory_to_use * + (total - reserving)); - PADDLE_ENFORCE_LE(allocating_memory, available_memory); + PADDLE_ENFORCE_LE(allocating, available, + "Insufficient GPU memory to allocation."); - return allocating_memory; + return allocating; } void GpuMemcpyAsync(void *dst, const void *src, size_t count,