Skip to content
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.

Sign up for GitHub

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

[gTest] Add more test coverage for Batchnorm Forward Training #3444

Merged
merged 7 commits into from
Jan 14, 2025
Merged

[gTest] Add more test coverage for Batchnorm Forward Training #3444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
82756e5
Add fwd training data for fwd spatial multiple variant = 2 and bfp32
xinlipn Dec 16, 2024
310192d
Add more test cases for forward spatial
xinlipn Dec 18, 2024
1ad4394
Merge branch 'develop' into sl/bn_fwd_training
xinlipn Dec 18, 2024
58458d8
Merge branch 'develop' into sl/bn_fwd_training
xinlipn Dec 18, 2024
94a7fd7
Replaced NetworkSmall with NetworkLarge for GPU_BN_FWD_Train_Large_FP64
xinlipn Dec 18, 2024
f0ab4bc
Merge branch 'develop' into sl/bn_fwd_training
xinlipn Jan 6, 2025
258b925
Merge branch 'develop' into sl/bn_fwd_training
xinlipn Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/gtest/bn_fwd_train.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ INSTANTIATE_TEST_SUITE_P(Smoke,

INSTANTIATE_TEST_SUITE_P(Smoke,
GPU_BN_FWD_Train_Large_FP64,
testing::Combine(testing::ValuesIn(NetworkSmall<BNTestCase>()),
testing::Combine(testing::ValuesIn(NetworkLarge<BNTestCase>()),
Comment on lines 145 to +147
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall it be a new Full instance? Let's keep Smoke tests relatively small, that's the point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @CAHEK7 could you clarify what you mean by "Full" instance? For each data type, there's 4 sets of test data for NetworkSmall and 32 for NetworkLarge.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a new Full instance. Its just there was a typo before. It should be calling NetworkLarge for testing AP2. For AP1 we only test small bn network (4 test).

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean Full tests and Smoke tests - Smoke is a small and fast subset which checks only a basic functionality and must not have any long-running tests or large configs.
The main point of Smoke tests and a quick sanity check before the long runs.

That's why I'm kind of recommend to move those tests into the Full tests, since it uses large configs.

testing::ValuesIn({miopenTensorNCHW, miopenTensorNHWC}),
testing::ValuesIn({testBNAPIV2})),
TestNameGenerator());
6 changes: 5 additions & 1 deletion test/gtest/bn_test_data.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ inline std::vector<BNTestCase> NetworkLarge()
{64, 64, 112, 112, miopenBNSpatial, miopen::batchnorm::Direction::ForwardInference, 1, 0},
{64, 64, 56, 56, miopenBNSpatial, miopen::batchnorm::Direction::Backward, 0, 1},
{64, 64, 56, 56, miopenBNSpatial, miopen::batchnorm::Direction::ForwardTraining, 1, 1},
{64, 64, 56, 56, miopenBNSpatial, miopen::batchnorm::Direction::ForwardInference, 1, 0}};
{64, 64, 56, 56, miopenBNSpatial, miopen::batchnorm::Direction::ForwardInference, 1, 0},
{768, 1, 14, 14, miopenBNSpatial, miopen::batchnorm::Direction::ForwardTraining, 1, 1},
{768, 1, 23, 23, miopenBNSpatial, miopen::batchnorm::Direction::ForwardTraining, 1, 1},
{832, 1, 14, 14, miopenBNSpatial, miopen::batchnorm::Direction::ForwardTraining, 1, 1},
{832, 1, 28, 28, miopenBNSpatial, miopen::batchnorm::Direction::ForwardTraining, 1, 1}};
}

template <>
Expand Down