Skip to content

Commit cf070e7

Browse files
bghimireamdclaude
andauthored
fix(hipdnn): wire golden harness into TOML tolerance + skip chain (#8738)
JIRA ID: ALMIOPEN-1969 ## Summary Extract shared `TomlGuards.hpp` and wire the non-golden and golden GPU harnesses through it so TOML-based tolerance overrides and test skips work uniformly. Fix `toleranceForNodeAttributes` missing SDPA cases. ## Motivation Tolerance-override and test-skip logic was duplicated in the non-golden harness and absent from the golden GPU harness (`IntegrationGraphGoldenReferenceVerificationHarness`). TOML `[[test_skips]]` and `[[tolerance_overrides]]` entries were silently ignored for bundle tests. Additionally, `toleranceForNodeAttributes` had no `SdpaAttributes`/`SdpaBackwardAttributes` cases, so SDPA bundles fell through to the 1e-3 default instead of using `tol::sdpa::getToleranceFwd<T>()`. ## Technical Details - **New `TomlGuards.hpp`** -- three shared free functions: `currentTestName()`, `applyTomlToleranceOverride()`, `checkTomlSkip()`. - **Non-golden harness** -- replaced inline skip/override blocks with `TomlGuards` calls (net -25 lines). `checkTomlSkip()` is inlined in `SetUp()` so `GTEST_SKIP()` returns from `SetUp()` directly. - **Golden GPU harness** -- added `checkTomlSkip()` inline in `SetUp()` before `applyMetadataGuards()`. TOML tolerance override resolved once before the per-tensor comparison loop in `compareEach()`, applied per-tensor after `resolveTolerances()`. - **SDPA tolerance** -- added `SdpaAttributes`/`SdpaBackwardAttributes` cases to `toleranceForNodeAttributes<T>()`. ### Why `TestGoldenReferenceCpu` is intentionally excluded `TestGoldenReferenceCpu` validates the CPU reference executor against golden data — it does not run the GPU engine. The TOML `[[test_skips]]` and `[[tolerance_overrides]]` are engine-specific (per-arch, per-engine). Applying them here would mask CPU reference regressions: if the oracle drifts from golden data, every GPU engine test that falls back to the CPU reference (auto mode) silently compares against a broken oracle and passes green. The CPU reference is deterministic and must always run with its hardcoded per-op tolerances. ## Test Plan - [x] Existing `TestTestConfig`, `TestTestSettings`, `TestVerificationModePaths`, `TestGoldenVerificationHarness`, `TestSynthesisTracker`, `TestSynthesizeInputs` all pass - [x] Pre-commit hooks clean (clang-format, cmake-lint) - [x] Bundle tests still gated behind `--allow-bundles` ## Test Result All unit tests pass. ## Submission Checklist - [x] Follows [contributing guidelines](https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests) - [x] No new warnings - [x] All existing tests pass cat: cd: No such file or directory cat: /home/AMD/bghimire/worktrees/rocm-libraries-wt-1969: Is a directory --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 329358b commit cf070e7

6 files changed

Lines changed: 168 additions & 25 deletions

File tree

dnn-providers/integration-tests/src/harness/IntegrationGraphVerificationHarness.hpp

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "harness/SharedHandle.hpp"
3131
#include "harness/SupportMatrixCollector.hpp"
3232
#include "harness/TestConfig.hpp"
33+
#include "harness/TomlGuards.hpp"
3334

3435
namespace hipdnn_integration_tests
3536
{
@@ -58,15 +59,9 @@ class IntegrationGraphVerificationHarness : public ::testing::TestWithParam<Test
5859
ASSERT_EQ(hipInit(0), hipSuccess);
5960
ASSERT_EQ(hipGetDevice(&_deviceId), hipSuccess);
6061

61-
// Check for any engine specific test skips
62-
if(auto* info = ::testing::UnitTest::GetInstance()->current_test_info(); info != nullptr)
62+
if(auto reason = checkTomlSkip(currentTestName()))
6363
{
64-
const std::string testName = std::string(info->test_suite_name()) + "." + info->name();
65-
if(auto skipReason = TestConfig::get().findSkipForTest(testName))
66-
{
67-
GTEST_SKIP() << "[arch " << TestConfig::get().getCurrentArch() << "] "
68-
<< *skipReason;
69-
}
64+
GTEST_SKIP() << "[arch " << TestConfig::get().getCurrentArch() << "] " << *reason;
7065
}
7166
}
7267

@@ -254,25 +249,9 @@ class IntegrationGraphVerificationHarness : public ::testing::TestWithParam<Test
254249
float absoluteTolerance,
255250
float relativeTolerance)
256251
{
257-
// Check for per-test tolerance override from TOML config
258252
float finalAtol = absoluteTolerance;
259253
float finalRtol = relativeTolerance;
260-
261-
auto* testInfo = ::testing::UnitTest::GetInstance()->current_test_info();
262-
if(testInfo != nullptr)
263-
{
264-
std::string testName
265-
= std::string(testInfo->test_suite_name()) + "." + testInfo->name();
266-
auto override = TestConfig::get().findToleranceOverride(testName);
267-
if(override.has_value())
268-
{
269-
finalAtol = override->atol;
270-
finalRtol = override->rtol;
271-
HIPDNN_PLUGIN_LOG_INFO("Tolerance override applied for " << testName
272-
<< ": atol=" << finalAtol
273-
<< " rtol=" << finalRtol);
274-
}
275-
}
254+
applyTomlToleranceOverride(currentTestName(), finalAtol, finalRtol);
276255

277256
// Since the graph can infer properties + Ids, we defer validator registration until right
278257
// before validation in verifyGraph
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// Copyright © Advanced Micro Devices, Inc., or its affiliates.
2+
// SPDX-License-Identifier: MIT
3+
4+
#pragma once
5+
6+
#include <optional>
7+
#include <string>
8+
9+
#include <gtest/gtest.h>
10+
11+
#include <hipdnn_plugin_sdk/PluginLogging.hpp>
12+
13+
#include "harness/TestConfig.hpp"
14+
15+
namespace hipdnn_integration_tests
16+
{
17+
18+
inline std::string currentTestName()
19+
{
20+
auto* info = ::testing::UnitTest::GetInstance()->current_test_info();
21+
if(info == nullptr)
22+
{
23+
return {};
24+
}
25+
return std::string(info->test_suite_name()) + "." + info->name();
26+
}
27+
28+
inline bool applyTomlToleranceOverride(const std::string& testName, float& atol, float& rtol)
29+
{
30+
if(testName.empty())
31+
{
32+
return false;
33+
}
34+
auto ovr = TestConfig::get().findToleranceOverride(testName);
35+
if(!ovr)
36+
{
37+
return false;
38+
}
39+
atol = ovr->atol;
40+
rtol = ovr->rtol;
41+
HIPDNN_PLUGIN_LOG_INFO("Tolerance override applied for " << testName << ": atol=" << atol
42+
<< " rtol=" << rtol);
43+
return true;
44+
}
45+
46+
inline std::optional<std::string> checkTomlSkip(const std::string& testName)
47+
{
48+
if(testName.empty())
49+
{
50+
return std::nullopt;
51+
}
52+
return TestConfig::get().findSkipForTest(testName);
53+
}
54+
55+
} // namespace hipdnn_integration_tests

dnn-providers/integration-tests/src/harness/golden/IntegrationGraphGoldenReferenceVerificationHarness.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "harness/ReferenceCapabilityError.hpp"
2525
#include "harness/SharedHandle.hpp"
2626
#include "harness/TestConfig.hpp"
27+
#include "harness/TomlGuards.hpp"
2728
#include "harness/golden/UnverifiableBundleReport.hpp"
2829
#include "harness/golden/input_init/SynthesizeInputs.hpp"
2930
#include "harness/gpu_graph_executor/GpuReferenceGraphExecutor.hpp"
@@ -490,6 +491,14 @@ void IntegrationGraphGoldenReferenceVerificationHarness::compareEach(OutputTenso
490491
auto wrapper = _bundle->graphWrapper();
491492
const auto& tensorAttrMap = wrapper.getTensorMap();
492493

494+
const auto tomlOverride = TestConfig::get().findToleranceOverride(currentTestName());
495+
if(tomlOverride)
496+
{
497+
HIPDNN_PLUGIN_LOG_INFO("Tolerance override applied for " << currentTestName()
498+
<< ": atol=" << tomlOverride->atol
499+
<< " rtol=" << tomlOverride->rtol);
500+
}
501+
493502
for(const int64_t uid : _bundle->outputTensorUids)
494503
{
495504
auto& actualTensor = *engineOutputs.at(uid);
@@ -502,6 +511,12 @@ void IntegrationGraphGoldenReferenceVerificationHarness::compareEach(OutputTenso
502511
float rtol = 0.0f;
503512
resolveTolerances(wrapper, dataType, atol, rtol);
504513

514+
if(tomlOverride)
515+
{
516+
atol = tomlOverride->atol;
517+
rtol = tomlOverride->rtol;
518+
}
519+
505520
compareOutputTensor(uid, *attrs, dataType, expectedTensor, actualTensor, atol, rtol);
506521
}
507522
}
@@ -672,6 +687,10 @@ float IntegrationGraphGoldenReferenceVerificationHarness::toleranceForNodeAttrib
672687
return tol::pointwise::getTolerance<T>();
673688
case NA::LayernormAttributes:
674689
return tol::layernorm::getTolerance<T>();
690+
case NA::SdpaAttributes:
691+
case NA::SdpaBackwardAttributes:
692+
// No backward golden tests yet; share forward tolerance until data exists
693+
return tol::sdpa::getToleranceFwd<T>();
675694
default:
676695
return 1e-3f;
677696
}

dnn-providers/integration-tests/src/harness/golden/IntegrationGraphGoldenReferenceVerificationHarness.hpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "harness/IReferenceGraphExecutor.hpp"
2323
#include "harness/TestConfig.hpp"
24+
#include "harness/TomlGuards.hpp"
2425
#include "harness/golden/IntegrationTestBundle.hpp"
2526

2627
namespace hipdnn_integration_tests::golden
@@ -53,6 +54,16 @@ using OutputTensors
5354
// are mark*Modified().
5455
// * Virtual (inter-node) tensors are allocated internally by each executor; the
5556
// variant packs we build carry only real (input + output) tensors.
57+
//
58+
// TODO(ALMIOPEN-1969 follow-up): Unify graph-init with the non-golden harness.
59+
// Stage 1 — Route non-golden ops whose initializeBundle() is plain randomize
60+
// (conv, matmul, BN-inference, reduction, rmsnorm-fwd, layernorm,
61+
// pointwise) through the synthesis switch. Zero behavioral change.
62+
// Stage 2 — Migrate structured recipes one op at a time: copy the exact
63+
// ranges/seeds/derivation from each non-golden subclass override
64+
// into the corresponding fill function, using fillComputed/tensorAt
65+
// for derived inputs. Delete each override once its fill fn works.
66+
// Stage 3 — Both harnesses share one init pipeline via SynthesisTracker.
5667
class IntegrationGraphGoldenReferenceVerificationHarness : public ::testing::Test
5768
{
5869
public:
@@ -81,6 +92,11 @@ class IntegrationGraphGoldenReferenceVerificationHarness : public ::testing::Tes
8192
GTEST_SKIP() << "No bundle set";
8293
}
8394

95+
if(auto reason = checkTomlSkip(currentTestName()))
96+
{
97+
GTEST_SKIP() << "[arch " << TestConfig::get().getCurrentArch() << "] " << *reason;
98+
}
99+
84100
applyMetadataGuards();
85101
}
86102

@@ -244,6 +260,9 @@ class IntegrationGraphGoldenReferenceVerificationHarness : public ::testing::Tes
244260
static std::string dataTypeName(hipdnn_flatbuffers_sdk::data_objects::DataType dataType);
245261

246262
// ── tolerances ──────────────────────────────────────────────────────
263+
// Two-level lookup: per-operation default from TestTolerances.hpp,
264+
// then TOML per-engine override (if a [[tolerance_overrides]] filter
265+
// matches the current gtest name).
247266
static void
248267
resolveTolerances(const hipdnn_flatbuffers_sdk::flatbuffer_utilities::GraphWrapper& wrapper,
249268
hipdnn_flatbuffers_sdk::data_objects::DataType dataType,

dnn-providers/integration-tests/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_executable(hipdnn_integration_tests_unit_tests
1212
TestSupportMatrixCollector.cpp
1313
TestTestConfig.cpp
1414
TestTestSettings.cpp
15+
TestTomlGuards.cpp
1516
TestGpuReferenceGraphExecutor.cpp
1617
TestGpuConvolutionFwdSignatureKey.cpp
1718
TestGpuConvolutionFwdPlan.cpp
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright © Advanced Micro Devices, Inc., or its affiliates.
2+
// SPDX-License-Identifier: MIT
3+
4+
#include <gtest/gtest.h>
5+
6+
#include "harness/TomlGuards.hpp"
7+
8+
using hipdnn_integration_tests::applyTomlToleranceOverride;
9+
using hipdnn_integration_tests::checkTomlSkip;
10+
using hipdnn_integration_tests::currentTestName;
11+
12+
// NOLINTBEGIN(readability-identifier-naming) -- gtest macro-generated names
13+
14+
// ---------------------------------------------------------------------------
15+
// currentTestName — pure gtest, no TestConfig dependency
16+
// ---------------------------------------------------------------------------
17+
18+
TEST(TestTomlGuards, NameReturnsExpectedFormat)
19+
{
20+
const auto name = currentTestName();
21+
EXPECT_EQ(name, "TestTomlGuards.NameReturnsExpectedFormat");
22+
}
23+
24+
TEST(TestTomlGuards, NameContainsDot)
25+
{
26+
const auto name = currentTestName();
27+
EXPECT_NE(name.find('.'), std::string::npos);
28+
}
29+
30+
// ---------------------------------------------------------------------------
31+
// checkTomlSkip / applyTomlToleranceOverride — empty-name early-return path
32+
// ---------------------------------------------------------------------------
33+
34+
TEST(TestTomlGuards, CheckTomlSkipReturnsNulloptForEmptyName)
35+
{
36+
EXPECT_EQ(checkTomlSkip(""), std::nullopt);
37+
}
38+
39+
TEST(TestTomlGuards, ApplyTomlToleranceOverrideReturnsFalseForEmptyName)
40+
{
41+
float atol = 1.0f;
42+
float rtol = 1.0f;
43+
EXPECT_FALSE(applyTomlToleranceOverride("", atol, rtol));
44+
EXPECT_FLOAT_EQ(atol, 1.0f);
45+
EXPECT_FLOAT_EQ(rtol, 1.0f);
46+
}
47+
48+
// ---------------------------------------------------------------------------
49+
// checkTomlSkip / applyTomlToleranceOverride — no TOML loaded
50+
//
51+
// TestConfig is initialized (by TestConfigInitialized in TestTestConfig.cpp,
52+
// same binary) without a settings file, so findSkipForTest / findToleranceOverride
53+
// return nullopt for any test name.
54+
// ---------------------------------------------------------------------------
55+
56+
TEST(TestTomlGuards, CheckTomlSkipReturnsNulloptWhenNoSettings)
57+
{
58+
EXPECT_EQ(checkTomlSkip("SomeTest.Name"), std::nullopt);
59+
}
60+
61+
TEST(TestTomlGuards, ApplyTomlToleranceOverrideReturnsFalseWhenNoSettings)
62+
{
63+
float atol = 1.0f;
64+
float rtol = 1.0f;
65+
EXPECT_FALSE(applyTomlToleranceOverride("SomeTest.Name", atol, rtol));
66+
EXPECT_FLOAT_EQ(atol, 1.0f);
67+
EXPECT_FLOAT_EQ(rtol, 1.0f);
68+
}
69+
70+
// NOLINTEND(readability-identifier-naming)

0 commit comments

Comments
 (0)