Skip to content

Support scalar-like analytics values#4683

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

Support scalar-like analytics values#4683
YuanTingHsieh wants to merge 1 commit 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 extends AnalyticsData to accept numpy scalar types (e.g., np.float32, 0-D arrays) by centralizing normalization in _normalize_numeric_scalar, and also fixes a latent bug where path=0 (a falsy non-string) was silently accepted instead of raising TypeError.

  • Adds _normalize_numeric_scalar static method that duck-types .item() + shape for scalar detection, with shape and exception guards, and applies it to SCALAR/METRIC values and dict values in METRICS/SCALARS (but not PARAMETERS, where string values are valid).
  • Fixes if path and ...if path is not None and ... so that a falsy non-string path (e.g., integer 0) is correctly rejected.
  • Tests cover generic scalar-like objects, numpy scalars/0-D arrays, dict normalization, non-scalar dict rejection, and the path=0 error-message fix; numpy tests use pytest.importorskip so numpy is optional.

Confidence Score: 5/5

Safe to merge — changes are additive, well-tested, and confined to a single validation/normalization layer with no impact on the serialization or transport path.

The normalization logic is straightforward duck-typing with explicit shape and exception guards; it does not change behavior for existing Python float/int inputs, only broadens acceptance. The path-guard fix is a correct tightening of a pre-existing silent acceptor. Tests cover all new paths including numpy-optional gating.

No files require special attention.

Important Files Changed

Filename Overview
nvflare/apis/analytix.py Adds _normalize_numeric_scalar static method and updates _validate_data_types to accept and normalize numpy-scalar-like values for SCALAR/METRIC types, normalize dict values for METRICS/SCALARS, and fixes the path is not None guard that previously silently accepted path=0.
tests/unit_test/apis/analytix_test.py Adds tests for generic scalar-like objects, numpy scalars/0-D arrays, METRICS/SCALARS dict normalization, non-scalar dict rejection, and the path=0 error-message fix; numpy tests use pytest.importorskip to avoid a hard dependency.
tests/unit_test/app_common/widgets/streaming_test.py Updates the expected error-message regex in INVALID_WRITE_TEST_CASES to match the new, more descriptive TypeError wording introduced for non-scalar SCALAR values.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["AnalyticsData init"] --> B["_validate_data_types"]
    B --> C{data_type}
    C -->|SCALAR or METRIC| D["_normalize_numeric_scalar"]
    D --> D1{float or int?}
    D1 -->|Yes| D2["return True, value"]
    D1 -->|No| D3{callable item?}
    D3 -->|No| D4["return False, value"]
    D3 -->|Yes| D5{shape == empty tuple?}
    D5 -->|No or TypeError| D4
    D5 -->|Yes or no shape attr| D6["call item()"]
    D6 --> D7{result float or int?}
    D7 -->|Yes| D8["return True, scalar"]
    D7 -->|No| D4
    D2 --> E["value = normalized_value"]
    D8 --> E
    D4 --> F["raise TypeError"]
    C -->|METRICS, PARAMETERS, or SCALARS| G{isinstance dict?}
    G -->|No| F
    G -->|Yes| H{METRICS or SCALARS?}
    H -->|No - PARAMETERS| I["accept dict as-is"]
    H -->|Yes| J["normalize each dict value"]
    J --> K{all values valid scalars?}
    K -->|No| F
    K -->|Yes| L["value = normalized_dict"]
    C -->|TEXT| M{isinstance str?}
    M -->|No| F
    M -->|Yes| N["accept"]
    E --> P["self.value = normalized value"]
    L --> P
    I --> P
    N --> P
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"] --> B["_validate_data_types"]
    B --> C{data_type}
    C -->|SCALAR or METRIC| D["_normalize_numeric_scalar"]
    D --> D1{float or int?}
    D1 -->|Yes| D2["return True, value"]
    D1 -->|No| D3{callable item?}
    D3 -->|No| D4["return False, value"]
    D3 -->|Yes| D5{shape == empty tuple?}
    D5 -->|No or TypeError| D4
    D5 -->|Yes or no shape attr| D6["call item()"]
    D6 --> D7{result float or int?}
    D7 -->|Yes| D8["return True, scalar"]
    D7 -->|No| D4
    D2 --> E["value = normalized_value"]
    D8 --> E
    D4 --> F["raise TypeError"]
    C -->|METRICS, PARAMETERS, or SCALARS| G{isinstance dict?}
    G -->|No| F
    G -->|Yes| H{METRICS or SCALARS?}
    H -->|No - PARAMETERS| I["accept dict as-is"]
    H -->|Yes| J["normalize each dict value"]
    J --> K{all values valid scalars?}
    K -->|No| F
    K -->|Yes| L["value = normalized_dict"]
    C -->|TEXT| M{isinstance str?}
    M -->|No| F
    M -->|Yes| N["accept"]
    E --> P["self.value = normalized value"]
    L --> P
    I --> P
    N --> P
Loading

Reviews (4): Last reviewed commit: "Support scalar-like analytics values" | 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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.42%. Comparing base (f35a4d4) to head (59f1e2b).

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.40%   56.42%   +0.01%     
==========================================
  Files         969      969              
  Lines       92192    92226      +34     
==========================================
+ Hits        52001    52036      +35     
+ Misses      40191    40190       -1     
Flag Coverage Δ
unit-tests 56.42% <84.61%> (+0.01%) ⬆️

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