Skip to content

[gTests] Create new gTests for bn_fwd_train and bn_bwd without saved mean#3582

Closed
xinlipn wants to merge 28 commits into
developfrom
sl/bntest_no_mean
Closed

[gTests] Create new gTests for bn_fwd_train and bn_bwd without saved mean#3582
xinlipn wants to merge 28 commits into
developfrom
sl/bntest_no_mean

Conversation

@xinlipn
Copy link
Copy Markdown
Contributor

@xinlipn xinlipn commented Mar 5, 2025

Create gTests to cover cases without saved Mean and saved InvVar, equivalent to e.g. the command below

./bin/MIOpenDriver bnormbfp16 -n 1 -c 2 -H 24 -W 16 -m 1 --forw 1 -b 0 -s 0 -r 0 -t 1 --layout NCHW

@xinlipn xinlipn marked this pull request as draft March 5, 2025 19:15
@xinlipn xinlipn self-assigned this Mar 5, 2025
@xinlipn xinlipn added the GTest label Mar 5, 2025
@bghimireamd
Copy link
Copy Markdown
Contributor

I do not see bn bwd cpu handle both "saved mean" and "no saved mean"

@xinlipn
Copy link
Copy Markdown
Contributor Author

xinlipn commented Mar 6, 2025

I do not see bn bwd cpu handle both "saved mean" and "no saved mean"

Hi @bghimireamd , that's a good catch. I have this change locally, but but bn_bwd doesn't seem to pass yet with it. I have just committed the change to ComputeCPUBNBwd

@xinlipn xinlipn requested a review from bghimireamd June 24, 2025 11:35
@xinlipn xinlipn marked this pull request as ready for review June 24, 2025 11:36
@xinlipn xinlipn changed the title [gTests] Create new gTests for bn_fwd_train and bn_bwd [gTests] Create new gTests for bn_fwd_train and bn_bwd without saved mean Jun 24, 2025
savedInvVar.generate(
uniform_signed_initializer<MeanVarDataType>(2e-3 /*scale*/, 1000 /*range*/));

if(this->saveMeanVar)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we comment here why the difference in initialization?

Copy link
Copy Markdown
Contributor Author

@xinlipn xinlipn Jun 26, 2025

Choose a reason for hiding this comment

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

can we comment here why the difference in initialization?

Hi @bghimireamd , MIOpenDriver command uses InitHostData to initialize the values, which is equivalent to the newly added fill_tensor_with_random_values, which create different initial values upon being called. In comparison, the generate() method will initialize tensors with the same initial values, which could lead to issues for no saved mean as I observed. But for this particular, i.e. input tensor, it may be ok using input.generate()

MIOpen/driver/bn_driver.hpp

Lines 266 to 269 in ea65f9d

bnScale.InitHostData(
bnScale.GetTensor().desc.GetElementSize(),
true,
uniform_signed_initializer<TScaleBias>(2e-3 /*scale*/, 1000 /*range*/));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

More details about the changes in how tensors are initialized.

if(this->saveMeanVar)
{
bnScale.generate(
uniform_signed_initializer<ScaleDataType>(2e-3 /*scale*/, 1000 /*range*/));
bnBias.generate(
uniform_signed_initializer<ScaleDataType>(2e-3 /*scale*/, 1000 /*range*/));
savedMean.generate(
uniform_signed_initializer<MeanVarDataType>(2e-3 /*scale*/, 1000 /*range*/));
savedInvVar.generate(
uniform_signed_initializer<MeanVarDataType>(2e-3 /*scale*/, 1000 /*range*/));
}
else
{
fill_tensor_with_random_values(bnScale, 2e-3 /*scale*/, 1000 /*range*/);
fill_tensor_with_random_values(bnBias, 2e-3 /*scale*/, 1000 /*range*/);
fill_tensor_with_random_values(savedMean, 2e-3 /*scale*/, 1000 /*range*/);
fill_tensor_with_random_values(savedInvVar, 2e-3 /*scale*/, 1000 /*range*/);
}

  1. Previous method calls generate(...) to initialize tensors, which seeds the RNG using a hash based on tensor dimensions desc.GetLenghths(), total number of elements data.size(), and Number of dimensions desc.GetLengths().size() as below. Thus, every time generated() is called on a tensor with the same shape, it will produce the same initial values

template <class G>
void generate_impl(G g)
{
auto seed = std::accumulate(desc.GetLengths().begin(),
desc.GetLengths().end(),
std::size_t{521288629},
[](auto x, auto y) {
x ^= x << 1U;
return x ^ y;
});
seed ^= data.size();
seed ^= desc.GetLengths().size();
prng::reset_seed(seed);

  1. In comparison, the new fill_tensor_with_random_values(...) uses std::generate(...) with a random generator function and no seed is explicitly set, the current RNG state may be based on a) Global RNG seed, b) System time, or c) previously changed state from other PRNG calls. It's non-deterministic and this way different tensors will have different initial values even for the same shape.

@amd-hsivasun
Copy link
Copy Markdown

Imported to ROCm/rocm-libraries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants