Conversation
Remove the duplicate NP item-digest wrapper and its standalone test.\n\nUse the existing sdigest::Item implementation as the sole item-digest path and clean up stale build entries and references.
Remove the unused template profiling helper and its unregistered example test.\n\nSProf remains the supported profiling implementation.
Remove the unused pointer-to-label cache, its explicit instantiation, and its standalone test.\n\nNo production callers remain.
Remove the unused byte-packing utility and its standalone test.\n\nNo production callers remain.
Remove the orphaned U4RandomTools header, which has no remaining callers or build references.
There was a problem hiding this comment.
🟡 Not ready to approve
SBnd::getItemDigest currently passes w as an array index into sdigest::Item, which changes behavior and can produce incorrect digests or out-of-bounds assertions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR removes a set of obsolete sysrap utilities (and their standalone tests / CMake registrations), and consolidates array-item digest computation onto the existing sdigest::Item implementation to eliminate stale references.
Changes:
- Deleted legacy utility headers/sources (
SDigestNP,SLabelCache,SPack,SProfile, plus some unused headers inu4,sysrap,qudarap) and removed associated tests. - Updated CMake source/header lists and test registrations to stop building removed code.
- Cleaned up digest call sites to use
sdigest::Item(but one call site currently misuses the API; see review comment).
File summaries
| File | Description |
|---|---|
| u4/U4RandomTools.hh | Removed deprecated random/Lambertian helper header. |
| u4/U4RandomDirection.hh | Removed deprecated random-direction helper header. |
| sysrap/tests/SProfile_test.h | Deleted obsolete SProfile test header. |
| sysrap/tests/SProfile_test.cc | Deleted obsolete SProfile test implementation. |
| sysrap/tests/SPackTest.cc | Deleted obsolete SPack test. |
| sysrap/tests/SLabelCacheTest.cc | Deleted obsolete SLabelCache test. |
| sysrap/tests/SDigestNPTest.cc | Deleted obsolete SDigestNP test. |
| sysrap/tests/SComponent_OLDTest.cc | Deleted obsolete/legacy component test. |
| sysrap/tests/CMakeLists.txt | Removed deleted tests from the test target. |
| sysrap/SSim.cc | Removed stale digest comment and uses sdigest::Item. |
| sysrap/squad_double.h | Removed unused dquad helper header. |
| sysrap/SProfile.h | Removed deprecated profiling utility header. |
| sysrap/SPack.hh | Removed deprecated packing utility header. |
| sysrap/SPack.cc | Removed deprecated packing utility implementation. |
| sysrap/SLabelCache.hh | Removed deprecated label-cache utility header. |
| sysrap/SLabelCache.cc | Removed deprecated label-cache explicit instantiation TU. |
| sysrap/SDigestNP.hh | Removed obsolete SDigestNP declaration. |
| sysrap/SDigestNP.cc | Removed obsolete SDigestNP implementation. |
| sysrap/SBnd.h | Updated digest calls (one needs correction). |
| sysrap/CMakeLists.txt | Removed deleted sources/headers from library build. |
| qudarap/qutil.h | Removed unused CUDA utility header. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| inline std::string SBnd::getItemDigest( int i, int j, int w ) const | ||
| { | ||
| return sdigest::Item(bnd, i, j, w ); // formerly SSim::GetItemDigest | ||
| return sdigest::Item(bnd, i, j, w); | ||
| } |
There was a problem hiding this comment.
🟢 Ready to approve
Repository-wide searches show no remaining references to the removed headers/symbols, and the remaining digest call sites consistently use sdigest::Item with explicit substring truncation.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Summary
sysraputilities:SDigestNP,SLabelCache,SPack, andSProfile.qudarap,sysrap, andu4.sdigest::Itemimplementation as the sole item-digest path and clean up stale references.This removes approximately 1,500 lines of unreferenced legacy code without changing supported functionality.