Skip to content

[PTSBE] fully qualify names and drop using namespace directives#4391

Open
taalexander wants to merge 5 commits intoNVIDIA:mainfrom
taalexander:fix/ptsbe-unittest-using-namespaces
Open

[PTSBE] fully qualify names and drop using namespace directives#4391
taalexander wants to merge 5 commits intoNVIDIA:mainfrom
taalexander:fix/ptsbe-unittest-using-namespaces

Conversation

@taalexander
Copy link
Copy Markdown
Collaborator

Address comments from @schweitzpgi that pointed out the style did not match the existing codebase.

@taalexander taalexander requested a review from schweitzpgi April 24, 2026 17:41
@taalexander taalexander force-pushed the fix/ptsbe-unittest-using-namespaces branch from 18c5b77 to a13243b Compare April 27, 2026 13:40
Several PTSBE test files and the Python binding piled up multiple
`using namespace` directives (cudaq + cudaq::ptsbe + cudaq::ptsbe::detail).
The house style is zero using-namespace directives with names fully
qualified at the use site. This removes every using-namespace directive
from unittests/ptsbe/*.cpp and python/runtime/cudaq/algorithms/py_sample_ptsbe.cpp
and qualifies each identifier against its owning namespace.

Behavior is unchanged; only name lookup is tightened.

Signed-off-by: Thomas Alexander <talexander@nvidia.com>
The previous commit's name-qualification pass accidentally prefixed
`cudaq::` inside two `#include` paths, yielding
`cudaq/ptsbe/cudaq::KrausSelection.h` and `cudaq/ptsbe/cudaq::KrausTrajectory.h`,
which broke the python binding build.

Signed-off-by: Thomas Alexander <talexander@nvidia.com>
…ions

Previous qualification pass placed these in the wrong sub-namespace:

- `countInstructions` and `numQubits` live in `cudaq::ptsbe`, not
  `cudaq::ptsbe::detail` (declared in PTSBEExecutionData.h).
- `sample_options` used in PTSBE tests is `cudaq::ptsbe::sample_options`
  (has a `.ptsbe` configuration sub-struct), not `cudaq::sample_options`.

Fixes compile errors in PTSBESampleTester.cpp and PTSBEMultiBackendTester.cpp.

Signed-off-by: Thomas Alexander <talexander@nvidia.com>
…_EPSILON

These identifiers are declared in `cudaq::` (KrausSelection.h and
KrausTrajectory.h open `namespace cudaq {...}`), not `cudaq::ptsbe::`.
A previous qualification pass wrongly placed them in `cudaq::ptsbe::`,
which broke KrausSelectionTester.cpp and KrausTrajectoryTester.cpp
because those TUs only include KrausSelection.h / KrausTrajectory.h
and never expose the `cudaq::ptsbe` namespace.

This also aligns with the convention noted in
.claude/rules/vendor-cuda-quantum.md and AGENTS.md: KrausSelection and
KrausTrajectory live in the `cudaq` namespace.

Signed-off-by: Thomas Alexander <talexander@nvidia.com>
The return type of buildAndExtract() was left as bare `detail::` after
the using-namespace drop. It needs `cudaq::ptsbe::detail::` since
NoiseExtractionResult is declared in `namespace cudaq::ptsbe::detail`
(see NoiseExtractor.h).

Verified with a local build + ctest of the qpp-backend PTSBE suite
(49/49 passing).

Signed-off-by: Thomas Alexander <talexander@nvidia.com>
@taalexander taalexander force-pushed the fix/ptsbe-unittest-using-namespaces branch from a13243b to 251b855 Compare April 28, 2026 14:13
Copy link
Copy Markdown
Collaborator

@schweitzpgi schweitzpgi left a comment

Choose a reason for hiding this comment

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

I think this can be a much smaller PR and just have focused on the files with more than 1

using namespace foo;
using namespace bar;
....

open namespace in the .cpp files.

For .h files, the number of open namespaces should be zero.

#include <nanobind/stl/unordered_map.h>
#include <nanobind/stl/vector.h>

using namespace cudaq;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FWIW, having one wide open namespace is fine. We just do not want more than one.


using namespace cudaq;
using namespace cudaq::ptsbe;
using namespace cudaq::ptsbe::detail;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Like here. You could leave

using namespace cudaq;

But we don't want the other 2.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants