From 99415f09fe86c4e078147c1130088facd93360b5 Mon Sep 17 00:00:00 2001 From: Scott McKay Date: Wed, 13 May 2020 07:18:32 +1000 Subject: [PATCH] Fix bug where linear_output_ is not cleared when linear_before_reset is true and no bias input is provided. Requires a batch size of 3 or more to trigger if initial_h is not provided. (#3893) --- .../core/providers/cpu/rnn/deep_cpu_gru.cc | 20 ++++---- onnxruntime/test/providers/cpu/rnn/GRU.py | 15 +++--- .../providers/cpu/rnn/deep_cpu_gru_op_test.cc | 47 ++++++++++++------- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/onnxruntime/core/providers/cpu/rnn/deep_cpu_gru.cc b/onnxruntime/core/providers/cpu/rnn/deep_cpu_gru.cc index 49ede2c72a..2a5b3d9f9c 100644 --- a/onnxruntime/core/providers/cpu/rnn/deep_cpu_gru.cc +++ b/onnxruntime/core/providers/cpu/rnn/deep_cpu_gru.cc @@ -526,22 +526,18 @@ void UniDirectionalGru::Compute(const gsl::span& inputs_arg, const int total_rows = max_sequence_length * batch_size_; float alpha = 1.0f; - float beta = 0.0f; // zero out outputZRH_ when calling ComputeGemm. // apply weights to all the inputs ComputeGemm(total_rows, hidden_size_x3, input_size_, alpha, inputs.cbegin(), inputs.cend(), input_size_, input_weights.cbegin(), input_weights.cend(), - input_size_, beta, + input_size_, 0.f, outputZRH_.begin(), outputZRH_.end(), hidden_size_x3, ttp_); DumpMatrix("inputs with weights applied", outputZRH_.data(), seq_length_ * batch_size_ * 3, hidden_size_); - // set to 1 so the weighted inputs in outputZRH_ are added to the result in the next call to ComputeGemm - beta = 1.0f; - // output shape is [seq_length, num_directions, batch_size, hidden_size] // if we are doing 2 directions and this is the forward pass we're writing to the real output so // need to include num_directions in the step length. @@ -599,7 +595,7 @@ void UniDirectionalGru::Compute(const gsl::span& inputs_arg, prev_Ht, prev_Ht_end, hidden_size_, recurrent_weightsZR.cbegin(), recurrent_weightsZR.cend(), - hidden_size_, beta, + hidden_size_, 1.f, // beta == 1 so we add existing values in outputZRH_ outputZRH_.begin() + out_added_offset, outputZRH_.end(), hidden_size_x3, ttp_); @@ -619,8 +615,10 @@ void UniDirectionalGru::Compute(const gsl::span& inputs_arg, prev_Ht, prev_Ht_end, // Ht-1 hidden_size_, recurrent_weightsH.cbegin(), recurrent_weightsH.cend(), // Rh^T - hidden_size_, beta, - linear_output_.begin(), linear_output_.end(), // pre: Rbh if use_bias_, post:output + hidden_size_, + use_bias_ ? 1.f : 0.f, // don't add values in linear_output_ if no bias input + linear_output_.begin(), + linear_output_.end(), // pre: Rbh if use_bias_, post:output hidden_size_, ttp_); DumpMatrix("Ht-1 * (Rh^T) + Rbh " + seqno_str, linear_output_.data(), batch_size_, hidden_size_); @@ -690,7 +688,7 @@ void UniDirectionalGru::Compute(const gsl::span& inputs_arg, cur_h_local, cur_h_local_end, // rt (.) Ht-1 hidden_size_, recurrent_weightsH.cbegin(), recurrent_weightsH.cend(), // Rh^T - hidden_size_, beta, + hidden_size_, 1.f, // beta == 1 to add Xt*(Wh^T) from out_H out_H, outputZRH_.end(), hidden_size_x3, ttp_); } @@ -830,9 +828,7 @@ void UniDirectionalGru::AllocateBuffers() { } if (linear_before_reset_) { - // if use_bias_ is true we copy bias values to this as the first use. if it's false we don't and need to initialize - bool fill = !use_bias_; - linear_output_ = Allocate(allocator_, batch_size_ * hidden_size_, linear_output_ptr_, fill); + linear_output_ = Allocate(allocator_, batch_size_ * hidden_size_, linear_output_ptr_); } auto batch_times_seq_length = batch_size_ * seq_length_; diff --git a/onnxruntime/test/providers/cpu/rnn/GRU.py b/onnxruntime/test/providers/cpu/rnn/GRU.py index 33542185a3..74ad0f20b5 100644 --- a/onnxruntime/test/providers/cpu/rnn/GRU.py +++ b/onnxruntime/test/providers/cpu/rnn/GRU.py @@ -260,17 +260,20 @@ class GRU_ONNXRuntimeUnitTests(): print_results(fw_output) @staticmethod - def BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows(linear_before_reset=0): + def BidirectionalDefaultActivationsSimpleWeightsNoBias(linear_before_reset=0): - print(GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows.__name__ + + print(GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBias.__name__ + '.linear_before_reset=' + str(linear_before_reset)) seq_length = 2 - batch_size = 2 + batch_size = 3 if linear_before_reset else 2 input_size = 1 hidden_size = 3 - input = np.array([[[1.], [2.]], [[10.], [11.]]]).astype(np.float32) + if linear_before_reset: + input = np.array([[[1.], [2.], [3.]], [[10.], [11.], [12.]]]).astype(np.float32) + else: + input = np.array([[[1.], [2.]], [[10.], [11.]]]).astype(np.float32) W = np.array([0.1, 0.2, 0.3, 1, 2, 3, 10, 11, 12]).astype(np.float32).reshape(1, 3 * hidden_size, input_size) @@ -384,8 +387,8 @@ class GRU_ONNXRuntimeUnitTests(): GRU_ONNXRuntimeUnitTests.ForwardDefaultActivationsSimpleWeightsNoBiasTwoRows() GRU_ONNXRuntimeUnitTests.ReverseDefaultActivationsSimpleWeightsNoBiasTwoRows() -GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows() -GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows(linear_before_reset=1) +GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBias() +GRU_ONNXRuntimeUnitTests.BidirectionalDefaultActivationsSimpleWeightsNoBias(linear_before_reset=1) GRU_ONNXRuntimeUnitTests.ForwardDefaultActivationsSimpleWeightsWithBiasBatchParallel() GRU_ONNXRuntimeUnitTests.ForwardDefaultActivationsSimpleWeightsWithBiasBatchParallelLinearBeforeReset() diff --git a/onnxruntime/test/providers/cpu/rnn/deep_cpu_gru_op_test.cc b/onnxruntime/test/providers/cpu/rnn/deep_cpu_gru_op_test.cc index b14b503191..7f53de617c 100644 --- a/onnxruntime/test/providers/cpu/rnn/deep_cpu_gru_op_test.cc +++ b/onnxruntime/test/providers/cpu/rnn/deep_cpu_gru_op_test.cc @@ -106,14 +106,21 @@ void DefaultActivationsSimpleWeightsNoBias(std::string direction, const std::vector& Y_h_data, bool linear_before_reset = false) { int64_t seq_length = 2; - int batch_size = 2; + int batch_size = linear_before_reset ? 3 : 2; // extra row to validate usage of linear_output_ int64_t input_size = 1; int64_t hidden_size = 3; int num_directions = direction == "bidirectional" ? 2 : 1; - std::vector X_data{1.f, 2.f, - 10.f, 11.f}; + std::vector X_data; + + if (linear_before_reset) { + X_data = {1.f, 2.f, 3.f, + 10.f, 11.f, 12.f}; + } else { + X_data = {1.f, 2.f, + 10.f, 11.f}; + } std::vector W_data{0.1f, 0.2f, 0.3f, // wz 1.f, 2.f, 3.f, // wr @@ -170,7 +177,7 @@ TEST(GRUTest, ReverseDefaultActivationsSimpleWeightsNoBiasTwoRows) { DefaultActivationsSimpleWeightsNoBias("reverse", Y_data, Y_h_data); } -TEST(GRUTest, BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows) { +TEST(GRUTest, BidirectionalDefaultActivationsSimpleWeightsNoBias) { std::vector Y_data{ // forward output for input sequence 0 0.4750208f, 0.450166f, 0.4255575f, @@ -200,32 +207,38 @@ TEST(GRUTest, BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRows) { DefaultActivationsSimpleWeightsNoBias("bidirectional", Y_data, Y_h_data); } -TEST(GRUTest, BidirectionalDefaultActivationsSimpleWeightsNoBiasTwoRowsLinearBeforeReset) { +TEST(GRUTest, BidirectionalDefaultActivationsSimpleWeightsNoBiasLinearBeforeReset) { std::vector Y_data{ // forward output for input sequence 0 - 0.47502081f, 0.450166f, 0.42555748f, - 0.450166f, 0.40131234f, 0.35434369f, + 0.4750208f, 0.450166f, 0.4255575f, + 0.45016602f, 0.40131235f, 0.35434368f, + 0.42555748f, 0.35434369f, 0.28905049f, // reverse output for input sequence 0 [sequence 1 in reversed input] - 0.60827853f, 0.50623393f, 0.4426924f, - 0.5803454f, 0.4527356f, 0.36886264f, + 0.6082785f, 0.50623393f, 0.4426924f, + 0.5803454f, 0.4527356f, 0.36886263f, + 0.5521325f, 0.40092295f, 0.30118297f, // forward output for input sequence 1 - 0.60270932f, 0.50830227f, 0.44950222f, - 0.57543688f, 0.45485455f, 0.37478411f, + 0.6027093f, 0.5083023f, 0.44950223f, + 0.5754369f, 0.45485455f, 0.3747841f, + 0.54791767f, 0.40301081f, 0.30608854f, // reverse output for input sequence 1 [sequence 0 in reversed input] - 0.26894142f, 0.11920292f, 0.04742587f, - 0.24973989f, 0.09975048f, 0.03557118f}; + 0.26894143f, 0.11920292f, 0.04742587f, + 0.24973989f, 0.09975048f, 0.03557118f, + 0.23147521f, 0.08317269f, 0.02659699f}; std::vector Y_h_data{ // we did the forward processing of input[1] last - 0.60270932f, 0.50830227f, 0.44950222f, - 0.57543688f, 0.45485455f, 0.37478411f, + 0.6027093f, 0.5083023f, 0.44950223f, + 0.5754369f, 0.45485455f, 0.3747841f, + 0.54791767f, 0.40301081f, 0.30608854f, // and the reverse processing of input[0] last as the input order was reversed - 0.60827853f, 0.50623393f, 0.4426924f, - 0.5803454f, 0.4527356f, 0.36886264f}; + 0.6082785f, 0.50623393f, 0.4426924f, + 0.5803454f, 0.4527356f, 0.36886263f, + 0.5521325f, 0.40092295f, 0.30118297f}; DefaultActivationsSimpleWeightsNoBias("bidirectional", Y_data, Y_h_data, true); }