Update vac_av files
#455
Merged
Update vac_av files
#455
+54
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removes vac_av.ipp, it was introduced when we used to have sr/mr interfaces and had a common vev method. Not needed anymore, this is cleaner. Adds include headers to vac_av.hpp
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the vac_av module to eliminate namespace ambiguity and improve code organization by converting the include-only implementation file (.ipp) to a proper source file (.cpp).
Key changes:
- Converts
vac_av.ipptovac_av.cppwith proper header guards and namespace structure - Removes nested
detailnamespace from header declarations, keeping it only in the implementation to resolve namespace ambiguity withop::detail - Removes duplicate
can_change_qnsfunction definition fromop.cpp
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| SeQuant/domain/mbpt/vac_av.hpp | Adds header guards, removes detail namespace from function declaration, adds proper namespace closing |
| SeQuant/domain/mbpt/vac_av.cpp | Converts from .ipp to .cpp, adds complete includes, implements functions in detail namespace, fixes typo in error message |
| SeQuant/domain/mbpt/op.hpp | Removes include of vac_av.hpp to break circular dependency |
| SeQuant/domain/mbpt/op.cpp | Removes include of vac_av.ipp and duplicate can_change_qns function |
| tests/unit/test_mbpt.cpp | Adds explicit include for vac_av.hpp |
| python/src/sequant/mbpt.h | Adds explicit include for vac_av.hpp |
| doc/examples/user/operator.cpp | Adds explicit include for vac_av.hpp |
| doc/examples/user/getting_started/ccd.cpp | Adds explicit include for vac_av.hpp |
| doc/examples/synopsis/synopsis6.cpp | Adds explicit include for vac_av.hpp |
| SeQuant/domain/mbpt/models/cc.hpp | Adds explicit include for vac_av.hpp |
| CMakeLists.txt | Updates build configuration to reference .cpp instead of .ipp |
Comments suppressed due to low confidence (1)
SeQuant/domain/mbpt/vac_av.cpp:146
- The typo "mpbt" has been corrected to "mbpt" in the error message, and the namespace path has been made more specific.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Doing this for consistency. Since op namespace is inlined, op::detail and mbpt::detail is ambiguous.
evaleev
approved these changes
Dec 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vac_av.cppvac_avfiles are not included inopfiles anymore.opis an inlined namespace,mbpt::op::detailandop::detailare ambiguous.