Skip to content

[hipDNN] Migrate private code to detail folders & namespace#4216

Merged
SamuelReeder merged 24 commits intodevelopfrom
users/sareeder/detail-migration
Feb 10, 2026
Merged

[hipDNN] Migrate private code to detail folders & namespace#4216
SamuelReeder merged 24 commits intodevelopfrom
users/sareeder/detail-migration

Conversation

@SamuelReeder
Copy link
Copy Markdown
Contributor

@SamuelReeder SamuelReeder commented Feb 3, 2026

Motivation

We want to move private code into an internal (detail) area that can omit versioning for easier changes and unnecessary bumps.

Technical Details

Moves everything that meets the following criteria into a <subcomponent>::detail namespace and folder:

  1. Only used internally by the subcomponent or it's tests.
  2. No expected use-cases for a user or plugin developer to leverage.

Migrations:

Frontend (hipdnn_frontend::detail)

File Purpose
BackendLoggingHelpers.hpp Status code formatting utilities
BackendWrapper.hpp Plugin loading/backend management
HipdnnBackendInterface.hpp Backend singleton interface
ScopedHipdnnBackendDescriptor.hpp RAII wrapper for backend descriptors
TopologicalSortingUtils.hpp Graph node ordering utilities
Utilities.hpp Internal validation helpers

Test SDK (hipdnn_test_sdk::detail)

Category Files
Plan implementations *Plan.hpp (Convolution, Batchnorm, Matmul, Pointwise)
Signature keys *SignatureKey.hpp (plan caching/registry)
Interfaces IGraphNodePlanBuilder.hpp, IGraphNodePlanExecutor.hpp
Utilities CpuFpReferenceUtilities.hpp, FlatbufferTensorAttributesUtils.hpp, PlanUtils.hpp, PlanBuilderRegistry.hpp, ScopedExecute.hpp

Data SDK (hipdnn_data_sdk::logging::detail)

File Purpose
CallbackSink.hpp Internal spdlog sink implementation for callback logging

Plugin SDK

Nothing applicable found in the plugin_sdk.

Test Plan

  • Build & test hipDNN
  • Build & test dnn-providers plugins

Submission Checklist

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 91.13924% with 49 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...end/include/hipdnn_frontend/detail/GraphDetail.hpp 78.57% 8 Missing and 7 partials ⚠️
.../hipdnn/frontend/include/hipdnn_frontend/Graph.hpp 90.14% 0 Missing and 7 partials ⚠️
...rontend/node/BatchnormInferenceNodeVarianceExt.hpp 50.00% 0 Missing and 6 partials ⚠️
...de/hipdnn_frontend/node/BatchnormInferenceNode.hpp 54.55% 0 Missing and 5 partials ⚠️
...end/include/hipdnn_frontend/node/BatchnormNode.hpp 66.67% 0 Missing and 5 partials ⚠️
.../include/hipdnn_frontend/node/detail/Utilities.hpp 98.17% 3 Missing and 1 partial ⚠️
...ude/hipdnn_frontend/node/BatchnormBackwardNode.hpp 80.00% 0 Missing and 3 partials ⚠️
...ontend/include/hipdnn_frontend/node/MatmulNode.hpp 0.00% 0 Missing and 2 partials ⚠️
...end/include/hipdnn_frontend/node/PointwiseNode.hpp 0.00% 0 Missing and 1 partial ⚠️
...s/cpu_graph_executor/CpuReferenceGraphExecutor.hpp 90.91% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (76.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4216      +/-   ##
===========================================
+ Coverage    65.32%   65.33%   +0.01%     
===========================================
  Files         1579     1582       +3     
  Lines       242137   242134       -3     
  Branches     33920    33920              
===========================================
+ Hits        158163   158182      +19     
+ Misses       69952    69931      -21     
+ Partials     14022    14021       -1     
Flag Coverage Δ *Carryforward flag
hipBLAS 90.67% <ø> (ø) Carriedforward from 67d6cbd
hipBLASLt 43.62% <ø> (ø) Carriedforward from 67d6cbd
hipDNN 81.52% <91.14%> (+0.13%) ⬆️
hipFFT 56.68% <ø> (ø) Carriedforward from 67d6cbd
hipSPARSE 84.70% <ø> (ø) Carriedforward from 67d6cbd
rocBLAS 47.97% <ø> (ø) Carriedforward from 67d6cbd
rocFFT 48.57% <ø> (ø) Carriedforward from 67d6cbd
rocSOLVER 76.83% <ø> (ø) Carriedforward from 67d6cbd
rocSPARSE 71.53% <ø> (ø) Carriedforward from 67d6cbd

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...dnn/frontend/include/hipdnn_frontend/Utilities.hpp 80.33% <100.00%> (-13.96%) ⬇️
...e/hipdnn_frontend/detail/BackendLoggingHelpers.hpp 100.00% <ø> (ø)
.../include/hipdnn_frontend/detail/BackendWrapper.hpp 57.63% <ø> (ø)
...hipdnn_frontend/detail/CreateBackendDescriptor.hpp 86.05% <ø> (ø)
.../hipdnn_frontend/detail/HipdnnBackendInterface.hpp 100.00% <ø> (ø)
..._frontend/detail/ScopedHipdnnBackendDescriptor.hpp 87.84% <100.00%> (ø)
...lude/hipdnn_frontend/node/ConvolutionDgradNode.hpp 92.31% <ø> (ø)
...lude/hipdnn_frontend/node/ConvolutionFpropNode.hpp 96.80% <ø> (ø)
...lude/hipdnn_frontend/node/ConvolutionWgradNode.hpp 96.39% <ø> (ø)
...n_frontend/node/detail/TopologicalSortingUtils.hpp 97.71% <ø> (ø)
... and 48 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SamuelReeder SamuelReeder marked this pull request as ready for review February 4, 2026 18:10
@SamuelReeder SamuelReeder requested a review from a team as a code owner February 4, 2026 18:10
Copy link
Copy Markdown
Contributor

@jerehartAMD jerehartAMD left a comment

Choose a reason for hiding this comment

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

Some namespaces I found that are nested one level too deep:

  • Just added in Graph.hpp two blocks in hipdnn_frontend::graph::detail
  • In LoadGraphAndTensors.hpp, a namespace in hipdnn_test_sdk::utilities::detail

Comment thread projects/hipdnn/docs/CodingStyleAndNamingGuidelines.md Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/Utilities.hpp
Comment thread projects/hipdnn/data_sdk/include/hipdnn_data_sdk/logging/detail/CallbackSink.hpp Outdated
Copy link
Copy Markdown
Contributor

@adickin-amd adickin-amd left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for doing all these moves to detail. Just a few non blocking comments if you choose to address them.

Comment thread projects/hipdnn/test_sdk/tests/utilities/TestLoadGraphAndTensors.cpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/node/ConvolutionDgradNode.hpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/node/ConvolutionFpropNode.hpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/node/ConvolutionWgradNode.hpp Outdated
Comment thread projects/hipdnn/frontend/tests/TestGraph.cpp
Comment thread projects/hipdnn/frontend/tests/TestBackendLoggingHelpers.cpp Outdated
Comment thread projects/hipdnn/frontend/include/hipdnn_frontend/Utilities.hpp
Comment thread projects/hipdnn/samples/batchnorm/BnBackward.cpp
Comment thread projects/hipdnn/docs/CodingStyleAndNamingGuidelines.md
@adickin-amd
Copy link
Copy Markdown
Contributor

Summary

This is a well-structured refactoring PR that properly separates internal implementation code from the public API by introducing detail namespaces. The changes follow a consistent pattern across the codebase and align with common C++ library practices (similar to how the STL uses detail or __impl namespaces for internal components).

Strengths:

  • Consistent namespace organization across all affected components
  • Comprehensive documentation added to the coding guidelines
  • Sample code updated to use proper public APIs instead of internal interfaces
  • No functional changes - purely organizational refactoring that improves maintainability
  • Test code has been updated to reflect the new structure

Suggestions for improvement:

  1. Fix the typo "attribue" → "attribute" in GraphDetail.hpp (line 29)
  2. Update the documentation table to accurately reflect the data_sdk detail namespace location
  3. Document the API change in samples (hipdnnBackend()hipdnnCreate()/hipdnnDestroy()) in the PR description or CHANGELOG

Overall assessment: The PR is well-executed and achieves its stated goal of moving private implementation code into internal namespaces, which will facilitate easier maintenance and cleaner API versioning going forward.


Generated by Claude Code

@SamuelReeder SamuelReeder merged commit fc9cb45 into develop Feb 10, 2026
18 of 19 checks passed
@SamuelReeder SamuelReeder deleted the users/sareeder/detail-migration branch February 10, 2026 15:45
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.

5 participants