Fix domino data factors logic

[coreyjadams]

PhysicsNeMo Pull Request

Currently, if you have surface-only or volume-only data, it will fail when loading the scaling factors.

This should nsure training sets factors to none if not used, and doesn't look for them if not needed.

Description

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.
• The CHANGELOG.md is up to date with these changes.
• An issue is linked to this pull request.

Dependencies

Review Process

All PRs are reviewed by the PhysicsNeMo team before merging.

Depending on which files are changed, GitHub may automatically assign a maintainer for review.

We are also testing AI-based code review tools (e.g., Greptile), which may add automated comments with a confidence score.
This score reflects the AI’s assessment of merge readiness and is not a qualitative judgment of your work, nor is
it an indication that the PR will be accepted / rejected.

AI-generated feedback should be reviewed critically for usefulness.
You are not required to respond to every AI comment, but they are intended to help both authors and reviewers.
Please react to Greptile comments with 👍 or 👎 to provide feedback on their accuracy.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR fixes a bug in the load_scaling_factors() function that caused failures when loading surface-only or volume-only data configurations. Previously, the function unconditionally attempted to access both volume_fields and surface_fields keys from the scaling factors, which would fail if the data only contained one type.

Key Changes:

• Added conditional checks based on cfg.model.model_type before accessing scaling factor data
• Volume factors are only loaded when model_type is "volume" or "combined"
• Surface factors are only loaded when model_type is "surface" or "combined"
• Factors are set to None when not needed, avoiding KeyError exceptions
• Tensor conversion only happens when factors are not None

Impact:
The fix ensures the function works correctly with all three model types (volume, surface, and combined) and is consistent with how the DoMINODataPipe class handles optional factors (which already checks for None before using them).

Confidence Score: 5/5

• This PR is safe to merge with minimal risk - it's a targeted bug fix with clear logic
• The change is well-structured and addresses a specific bug without introducing new risks. The conditional logic correctly checks model_type before accessing scaling factors, and the None handling is consistent with the downstream DoMINODataPipe class that already expects optional factors. The code maintains backward compatibility for the "combined" model type while fixing the issue for "surface" and "volume" only configurations.
• No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
examples/cfd/external_aerodynamics/domino/src/utils.py	5/5	Added conditional logic to only load and process scaling factors for model types that need them, preventing failures when using surface-only or volume-only models

Sequence Diagram

sequenceDiagram
    participant Caller as Training Script
    participant LSF as load_scaling_factors()
    participant SF as ScalingFactors
    participant DM as DistributedManager
    participant Config as cfg.model
    
    Caller->>LSF: load_scaling_factors(cfg, logger)
    LSF->>SF: ScalingFactors.load(pickle_path)
    SF-->>LSF: scaling_factors object
    
    alt normalization == "min_max_scaling"
        LSF->>Config: Check model_type
        alt model_type == "volume" or "combined"
            LSF->>SF: Access max_val["volume_fields"], min_val["volume_fields"]
            SF-->>LSF: volume data
            LSF->>LSF: Create vol_factors array
        else model_type == "surface"
            LSF->>LSF: Set vol_factors = None
        end
        
        alt model_type == "surface" or "combined"
            LSF->>SF: Access max_val["surface_fields"], min_val["surface_fields"]
            SF-->>LSF: surface data
            LSF->>LSF: Create surf_factors array
        else model_type == "volume"
            LSF->>LSF: Set surf_factors = None
        end
    else normalization == "mean_std_scaling"
        Note over LSF,SF: Similar conditional logic for mean/std
    end
    
    LSF->>DM: Get device
    DM-->>LSF: device
    
    alt vol_factors is not None
        LSF->>LSF: Convert vol_factors to tensor and move to device
    else
        LSF->>LSF: Set vol_factors_tensor = None
    end
    
    alt surf_factors is not None
        LSF->>LSF: Convert surf_factors to tensor and move to device
    else
        LSF->>LSF: Set surf_factors_tensor = None
    end
    
    LSF-->>Caller: Return (vol_factors_tensor, surf_factors_tensor)

Loading

[greptile-apps]

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}