Skip to content

add method to return map sizes#4294

Merged
pinkenburg merged 1 commit into
sPHENIX-Collaboration:masterfrom
pinkenburg:cdbttree-size
Jun 11, 2026
Merged

add method to return map sizes#4294
pinkenburg merged 1 commit into
sPHENIX-Collaboration:masterfrom
pinkenburg:cdbttree-size

Conversation

@pinkenburg

@pinkenburg pinkenburg commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [X ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work for users)
  • Requiring change in macros repository (Please provide links to the macros pull request in the last section)
  • I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

In some cases one needs the number of entries in the maps. This PR adds methods which return those.

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

PR Summary: Map Size Accessor Methods for CDBTTree

Motivation & Context

This PR adds public accessor methods to the CDBTTree class, which is part of the sPHENIX Collaboration's Conditions Database Objects system. The CDBTTree class manages calibration parameters organized in internal maps by channel and parameter name, supporting four data types: float, double, int, and uint64_t. These new methods provide direct access to the number of entries stored in each data type's map, enabling users to query map sizes for validation, diagnostics, and data completeness checks.

Key Changes

  • Four new inline public accessor methods:
    • size_t GetFloatMapSize() const — returns count of float channel entries
    • size_t GetDoubleMapSize() const — returns count of double channel entries
    • size_t GetIntMapSize() const — returns count of int channel entries
    • size_t GetUInt64MapSize() const — returns count of uint64_t channel entries
  • Inline implementations in the header file for efficient O(1) access to map sizes
  • Minor comment updates in the implementation file (no functional changes)
  • Fully backward compatible — only adds new methods with no modifications to existing functionality or data structures

Potential Risk Areas

  • None identified: Changes are purely additive and do not affect existing code paths, data formats, or reconstruction behavior
  • Performance: Negligible impact; inline methods return pre-computed map sizes with O(1) complexity
  • Thread-safety: No new thread-safety considerations; inherits existing patterns from CDBTTree class

Context

The underlying maps (e.g., m_FloatEntryMap, m_DoubleEntryMap) are nested std::map structures where the outer map key is a channel ID and inner maps contain parameter names to values. The new accessors expose only the outer map sizes, providing a convenient way to check how many channels have calibration data without exposing implementation details.

Note: As per collaboration guidelines, AI-generated analysis may contain inaccuracies; code review should verify implementation details directly.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds four public const accessor methods to CDBTTree for querying internal map sizes (GetFloatMapSize, GetDoubleMapSize, GetIntMapSize, GetUInt64MapSize), and includes commented-out contains guards in four setter methods as inline documentation of the underlying entry maps.

Changes

CDBTTree map size accessors and documentation

Layer / File(s) Summary
Map size accessor methods
offline/database/cdbobjects/CDBTTree.h
Public header declares four inline const methods returning size_t for the sizes of float, double, int, and uint64 entry maps.
Setter method entry map documentation
offline/database/cdbobjects/CDBTTree.cc
SetSingleFloatValue, SetSingleDoubleValue, SetSingleIntValue, and SetSingleUInt64Value each gain a commented-out contains guard line documenting the entry map accessed; no executable logic changes.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
offline/database/cdbobjects/CDBTTree.cc

offline/database/cdbobjects/CDBTTree.cc:3:10: fatal error: 'phool/phool.h' file not found
3 | #include <phool/phool.h>
| ^~~~~~~~~~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/bd113267254a3754b7718efe222573c69b054c81-381105df6838f3f4/tmp/clang_command_.tmp.f027fe.txt
++Contents of '/tmp/coderabbit-infer/bd113267254a3754b7718efe222573c69b054c81-381105df6838f3f4/tmp/clang_command_.tmp.f027fe.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-triple"
"x86_64-unknown-linux-gnu" "-emit-obj" "-mrelax-all" "

... [truncated 1090 characters] ...

clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-fdeprecated-macro" "-ferror-limit" "19" "-fgnuc-version=4.2.1"
"-fskip-odr-check-in-gmf" "-fcxx-exceptions" "-fexceptions"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/381105df6838f3f4/file.o" "-x" "c++"
"offline/database/cdbobjects/CDBTTree.cc" "-O0" "-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 00692e6b-a1c1-42b0-aa01-1f90d2902c2d

📥 Commits

Reviewing files that changed from the base of the PR and between 166a9a5 and bd11326.

📒 Files selected for processing (2)
  • offline/database/cdbobjects/CDBTTree.cc
  • offline/database/cdbobjects/CDBTTree.h

{
std::string fieldname = "g" + name;
// if (m_SingleUInt64EntryMap.contains(fieldname))
// if (m_SingleUInt64EntryMap.contains(fieldname))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing negation operator in commented contains() check.

The commented line should read if (!m_SingleUInt64EntryMap.contains(fieldname)) to match the active logic on line 292, which checks whether the key is not found. The other three setter methods (lines 233, 252, 271) correctly include the ! operator in their commented equivalents.

🔧 Proposed fix
-  //  if (m_SingleUInt64EntryMap.contains(fieldname))
+  //  if (!m_SingleUInt64EntryMap.contains(fieldname))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// if (m_SingleUInt64EntryMap.contains(fieldname))
// if (!m_SingleUInt64EntryMap.contains(fieldname))

@sphenix-jenkins-ci

Copy link
Copy Markdown

Build & test report

Report for commit bd113267254a3754b7718efe222573c69b054c81:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg

Copy link
Copy Markdown
Contributor Author

jenkins seems stuck, merging this now

@pinkenburg pinkenburg merged commit 3d98726 into sPHENIX-Collaboration:master Jun 11, 2026
20 of 22 checks passed
@pinkenburg pinkenburg deleted the cdbttree-size branch June 11, 2026 13:55
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.

1 participant