Skip to content

Support scalar-like analytics values#4683

Open
YuanTingHsieh wants to merge 2 commits into
NVIDIA:mainfrom
YuanTingHsieh:codex/analytics-numpy-scalars-v2
Open

Support scalar-like analytics values#4683
YuanTingHsieh wants to merge 2 commits into
NVIDIA:mainfrom
YuanTingHsieh:codex/analytics-numpy-scalars-v2

Conversation

@YuanTingHsieh

@YuanTingHsieh YuanTingHsieh commented May 22, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • Normalize scalar-like analytics values when constructing AnalyticsData.
  • Accept Python int/float values as before, plus scalar-like objects with .item() such as numpy.float32 and 0-D numpy arrays.
  • Normalize numeric values inside SCALARS and METRICS dictionaries, and fail fast if a dict entry is not numeric-scalar-like.
  • Fix the invalid path validation message to report type(path).
  • Add unit coverage for generic scalar-like objects, numpy scalar / 0-D array values, METRICS dict normalization, non-scalar dict rejection, and the path error message.

Why

Client tracking writers currently reject values such as numpy.float32 for single scalar metrics because analytics validation only accepts built-in Python float/int. This can happen with code like:

running_loss += cost.cpu().detach().numpy() / images.size()[0]

Centralizing the normalization in AnalyticsData keeps SummaryWriter, MLflowWriter, WandBWriter, and lower-level analytics logging paths consistent without adding a numpy dependency to core API code.

This is a replacement PR for the previous glitched PR #4659, rebased onto the latest NVIDIA/NVFlare:main before branch creation.

@YuanTingHsieh YuanTingHsieh changed the title [codex] Support scalar-like analytics values Support scalar-like analytics values May 22, 2026
@YuanTingHsieh YuanTingHsieh force-pushed the codex/analytics-numpy-scalars-v2 branch from 7efa912 to b2f76b7 Compare May 22, 2026 22:18
@YuanTingHsieh YuanTingHsieh marked this pull request as ready for review May 22, 2026 22:18
Copilot AI review requested due to automatic review settings May 22, 2026 22:18

Copilot AI 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.

Pull request overview

This PR updates the AnalyticsData validation layer to accept and normalize “scalar-like” numeric values (e.g., objects with .item() such as NumPy scalars / 0-D arrays), and extends unit tests to cover the new behavior and improved error messaging.

Changes:

  • Normalize scalar-like values for SCALAR/METRIC analytics values during AnalyticsData construction.
  • Normalize numeric entries inside SCALARS/METRICS dict payloads and reject non-numeric-scalar dict values.
  • Add unit tests for scalar-like normalization (including NumPy), dict normalization/rejection, and the path validation error message.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
nvflare/apis/analytix.py Adds numeric-scalar normalization logic and applies it to scalar and dict-based analytics value validation; fixes path error message type reporting.
tests/unit_test/apis/analytix_test.py Adds unit coverage for scalar-like and NumPy scalar normalization, dict normalization/rejection, and path error message validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread nvflare/apis/analytix.py Outdated
Comment thread nvflare/apis/analytix.py
Comment thread tests/unit_test/apis/analytix_test.py Outdated
@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR allows analytics tracking to accept scalar-like objects (e.g. numpy.float32, 0-D numpy arrays) in addition to plain Python float/int, normalising them to built-in Python types via a new _normalize_numeric_scalar helper. It also normalises dictionary values for METRICS and SCALARS data types and fixes a pre-existing bug where a falsy-but-not-None path argument (e.g. path=0) bypassed type validation.

  • _validate_data_types now returns the (possibly normalised) value, which is then stored in self.value, keeping all four writer backends consistent without adding a numpy dependency to the core API.
  • Dict normalisation is applied only to METRICS/SCALARS (not PARAMETERS), matching the expectation that parameter values can be arbitrary types.
  • Tests cover generic scalar-like objects, numpy scalars, 0-D arrays, dict normalisation, non-scalar dict rejection, and the corrected path error message.

Confidence Score: 5/5

Safe to merge — the change is additive and well-tested, with no regressions to existing validation paths.

The core normalization logic is self-contained in a new static method, the return-value threading through _validate_data_types is correct, and the path-validation fix is straightforward. All new code paths are exercised by the added tests, including numpy-specific and generic scalar-like cases.

No files require special attention. The _normalize_numeric_scalar static method in analytix.py is the only net-new logic worth a careful read.

Important Files Changed

Filename Overview
nvflare/apis/analytix.py Extends _validate_data_types to normalise scalar-like values for SCALAR/METRIC and normalise dict values for METRICS/SCALARS; also fixes a falsy-path guard bug where path=0 previously bypassed validation.
tests/unit_test/apis/analytix_test.py Adds five new test cases covering scalar-like normalisation, path error message, dict normalisation, non-scalar dict rejection, and numpy scalar/0-D array handling.
tests/unit_test/app_common/widgets/streaming_test.py Updates the error-message regex in an existing invalid-write test to match the new TypeError text; correct use of raw string for regex escaping.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["AnalyticsData.__init__(key, value, data_type)"] --> B["_validate_data_types()"]
    B --> C{data_type?}
    C -->|SCALAR or METRIC| D["_normalize_numeric_scalar(value)"]
    D --> D1{float or int?}
    D1 -->|Yes| D2["return value unchanged"]
    D1 -->|No| D3{has callable item method?}
    D3 -->|No| D4["return False - TypeError"]
    D3 -->|Yes| D5{has shape attribute?}
    D5 -->|shape not empty tuple| D4
    D5 -->|shape is empty tuple or absent| D6["call item()"]
    D6 --> D7{result is float or int?}
    D7 -->|Yes| D8["return normalized scalar"]
    D7 -->|No| D4
    D2 --> Z["self.value = normalized value"]
    D8 --> Z
    C -->|METRICS PARAMETERS or SCALARS| E{isinstance dict?}
    E -->|No| E1["TypeError"]
    E -->|Yes| F{METRICS or SCALARS?}
    F -->|No PARAMETERS| G["accept dict as-is"]
    F -->|Yes| H["normalize each dict entry"]
    H --> H1{all entries pass?}
    H1 -->|No| H2["TypeError for offending key"]
    H1 -->|Yes| H3["value = normalized dict"]
    G --> Z
    H3 --> Z
    C -->|TEXT| T{isinstance str?}
    T -->|No| T1["TypeError"]
    T -->|Yes| Z
    C -->|TAGS| U{isinstance dict?}
    U -->|No| U1["TypeError"]
    U -->|Yes| Z
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["AnalyticsData.__init__(key, value, data_type)"] --> B["_validate_data_types()"]
    B --> C{data_type?}
    C -->|SCALAR or METRIC| D["_normalize_numeric_scalar(value)"]
    D --> D1{float or int?}
    D1 -->|Yes| D2["return value unchanged"]
    D1 -->|No| D3{has callable item method?}
    D3 -->|No| D4["return False - TypeError"]
    D3 -->|Yes| D5{has shape attribute?}
    D5 -->|shape not empty tuple| D4
    D5 -->|shape is empty tuple or absent| D6["call item()"]
    D6 --> D7{result is float or int?}
    D7 -->|Yes| D8["return normalized scalar"]
    D7 -->|No| D4
    D2 --> Z["self.value = normalized value"]
    D8 --> Z
    C -->|METRICS PARAMETERS or SCALARS| E{isinstance dict?}
    E -->|No| E1["TypeError"]
    E -->|Yes| F{METRICS or SCALARS?}
    F -->|No PARAMETERS| G["accept dict as-is"]
    F -->|Yes| H["normalize each dict entry"]
    H --> H1{all entries pass?}
    H1 -->|No| H2["TypeError for offending key"]
    H1 -->|Yes| H3["value = normalized dict"]
    G --> Z
    H3 --> Z
    C -->|TEXT| T{isinstance str?}
    T -->|No| T1["TypeError"]
    T -->|Yes| Z
    C -->|TAGS| U{isinstance dict?}
    U -->|No| U1["TypeError"]
    U -->|Yes| Z
Loading

Reviews (5): Last reviewed commit: "Merge branch 'main' into codex/analytics..." | Re-trigger Greptile

Comment thread nvflare/apis/analytix.py Outdated
Comment thread nvflare/apis/analytix.py
@YuanTingHsieh YuanTingHsieh force-pushed the codex/analytics-numpy-scalars-v2 branch 2 times, most recently from b4c9ce8 to d66bbbe Compare May 22, 2026 23:57
@YuanTingHsieh YuanTingHsieh force-pushed the codex/analytics-numpy-scalars-v2 branch from 058b94a to 59f1e2b Compare June 30, 2026 22:29
@codecov-commenter

codecov-commenter commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.01%. Comparing base (4e4ce7e) to head (1830048).

Files with missing lines Patch % Lines
nvflare/apis/analytix.py 84.61% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4683      +/-   ##
==========================================
+ Coverage   56.98%   57.01%   +0.02%     
==========================================
  Files         969      969              
  Lines       92310    92344      +34     
==========================================
+ Hits        52606    52650      +44     
+ Misses      39704    39694      -10     
Flag Coverage Δ
unit-tests 57.01% <84.61%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@YuanTingHsieh YuanTingHsieh requested a review from holgerroth July 1, 2026 23:00
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.

3 participants