ONNX modifications with no op exit early

[klemen1999]

Purpose

• If ONNX model doesn't require any modifications then we just return the original model instead of loading the whole graph into memory. This because especially noticable with bigger models
• If there already exists <model name>.info.csv file from snpe-dlc-info and is not older than the dlc file itself then we use it. This way we save some time instead of always re-generating it

Specification

None / not applicable

Dependencies & Potential Impact

None / not applicable

Deployment Plan

None / not applicable

Testing & Validation

None / not applicable

AI Usage

Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]

Submitted code was reviewed by a human: YES/NO

The author is taking the responsibility for the contribution: YES/NO

Summary by CodeRabbit

• Refactor

  • Enhanced model conversion performance through metadata caching to avoid redundant operations.
  • Streamlined input normalization handling during ONNX processing.

• Tests

  • Expanded test coverage for model conversion edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR optimizes ONNX input normalization processing by introducing a centralized decision method that determines whether inputs require normalization changes, enabling the ONNX attachment function to short-circuit expensive model loading and rewriting operations. It also improves DLC metadata extraction performance through conditional caching and includes validation tests.

Changes

ONNX Input Normalization Optimization

Layer / File(s)	Summary
Input modification detection contract modelconverter/utils/config.py	New requires_onnx_input_modification method evaluates whether an input requires ONNX normalization based on encoding mismatch and mean/scale deviations from default values (mean=0, scale=1).
ONNX processing short-circuit modelconverter/utils/onnx_tools.py	Uses the new decision method to short-circuit model loading and rewriting when no inputs require normalization changes; per-input skip logic is unified to use the same decision method.
DLC metadata caching modelconverter/utils/metadata.py	Caches DLC metadata in .info.csv files and conditionally executes snpe-dlc-info only when the cache is missing or older than the source model.
Test infrastructure and validation tests/test_utils/test_config.py	Updates YAML construction in test utilities using dedent and adds test_modified_onnx_noop_returns_original_model to verify that ONNX processing returns the original model path when no normalization is needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• tersekmatija
• conorsim

Poem

🐰 Optimization hops with grace so neat,
Short-circuiting where inputs don't meet,
Caching metadata, avoiding the grind,
Leaving normalized changes behind,
Efficiency blooms—the change is complete! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main optimization: ONNX model modifications now exit early when no changes are needed, avoiding unnecessary memory usage.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/onnx_optimizations

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/test_utils/test_config.py (1)

843-864: ⚡ Quick win

Test validates the short-circuit behavior correctly.

The test confirms that when no ONNX input modifications are required (encoding set to NONE with no mean/scale overrides), the function returns the original model path and does not create a modified file.

Optional: Broaden test coverage for other neutral-value scenarios

Consider adding a test case for other "no modification needed" scenarios, such as:

• encoding.from == encoding.to (e.g., both BGR)
• mean_values = None (or explicitly [0, 0, 0])
• scale_values = None (or explicitly [1, 1, 1])

def test_modified_onnx_noop_with_matching_encoding():
    model_path = DATA_DIR / "dummy_model.onnx"
    modified_path = DATA_DIR / "dummy_model_noop_modified2.onnx"
    config = Config.get_config(
        None,
        {
            "input_model": str(model_path),
            "encoding.from": "BGR",
            "encoding.to": "BGR",
            "mean_values": None,
            "scale_values": None,
        },
    )
    input_configs = {
        inp.name: inp for inp in next(iter(config.stages.values())).inputs
    }

    result = onnx_attach_normalization_to_inputs(
        model_path,
        modified_path,
        input_configs,
    )

    assert result == model_path
    assert not modified_path.exists()

This would validate the short-circuit works for all neutral-value combinations, not just the NONE encoding case.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_utils/test_config.py` around lines 843 - 864, Add one or more
tests that assert the same short-circuit behavior for other neutral-value
scenarios besides encoding "NONE": create tests (e.g.,
test_modified_onnx_noop_with_matching_encoding) that call Config.get_config with
encoding.from and encoding.to set to the same value (like "BGR"), and with
mean_values set to None or [0,0,0] and scale_values set to None or [1,1,1]; then
call onnx_attach_normalization_to_inputs with the generated input_configs and
assert it returns the original model path and that the modified_path file does
not exist, reusing DATA_DIR, onnx_attach_normalization_to_inputs,
Config.get_config and the existing test pattern to keep coverage consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@modelconverter/utils/metadata.py`:
- Around line 53-55: The call to subprocess_run invoking "snpe-dlc-info"
(subprocess_run(["snpe-dlc-info", "-i", path, "-s", csv_path], silent=True)) can
hang indefinitely; add a timeout argument (e.g., timeout=some_seconds) to the
subprocess_run call and update the surrounding logic to catch the timeout
exception (e.g., subprocess.TimeoutExpired) so you can log/handle the failure
and proceed or fail gracefully instead of blocking archive generation; modify
the invocation in metadata extraction where subprocess_run is called and ensure
any cleanup or error logging uses the same path/csv_path context.

---

Nitpick comments:
In `@tests/test_utils/test_config.py`:
- Around line 843-864: Add one or more tests that assert the same short-circuit
behavior for other neutral-value scenarios besides encoding "NONE": create tests
(e.g., test_modified_onnx_noop_with_matching_encoding) that call
Config.get_config with encoding.from and encoding.to set to the same value (like
"BGR"), and with mean_values set to None or [0,0,0] and scale_values set to None
or [1,1,1]; then call onnx_attach_normalization_to_inputs with the generated
input_configs and assert it returns the original model path and that the
modified_path file does not exist, reusing DATA_DIR,
onnx_attach_normalization_to_inputs, Config.get_config and the existing test
pattern to keep coverage consistent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 27517f98-2c72-4baa-90ab-4e77d74ea154

📥 Commits

Reviewing files that changed from the base of the PR and between 959a32f and 5864a59.

📒 Files selected for processing (4)

• modelconverter/utils/config.py
• modelconverter/utils/metadata.py
• modelconverter/utils/onnx_tools.py
• tests/test_utils/test_config.py