Extend DataInputParams with IntensityConfig fields for model export#5964
Extend DataInputParams with IntensityConfig fields for model export#5964omkar-334 wants to merge 4 commits intoopen-edge-platform:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Extends the model preprocessing metadata pipeline so IntensityConfig settings (e.g., storage dtype and intensity mapping mode/parameters) can be carried through DataInputParams and embedded into exported model IR metadata (rt_info), enabling downstream consumers to reconstruct intensity handling at deployment time.
Changes:
- Added intensity-related fields to
DataInputParamsand updated dict conversion / preprocessing-param merging. - Propagated
IntensityConfigfrom theOTXDataModuleinto model instantiation paths (engine + CLI). - Embedded intensity-related values into exported model metadata and updated a unit test expectation for
DataInputParams.as_dict().
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
library/src/otx/backend/native/models/base.py |
Extends DataInputParams and merges intensity defaults into preprocessing params. |
library/src/otx/backend/native/exporter/base.py |
Writes intensity-related metadata into exported model rt_info. |
library/src/otx/data/module.py |
Exposes intensity_config from the train subset on the datamodule. |
library/src/otx/backend/native/engine.py |
Passes intensity fields from datamodule into data_input_params. |
library/src/otx/cli/cli.py |
Mirrors engine behavior for CLI-driven model instantiation. |
library/tests/unit/backend/native/models/test_base.py |
Updates DataInputParams.as_dict() unit test to include new fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Fall back to DataInputParams field defaults when model has no defaults. | ||
| fallback = DataInputParams(input_size=(0, 0), mean=(0.0, 0.0, 0.0), std=(1.0, 1.0, 1.0)) | ||
| default = default or fallback |
There was a problem hiding this comment.
In _configure_preprocessing_params, the comment says to fall back to DataInputParams field defaults, but the fallback instance hard-codes input_size=(0,0), mean=(0,0,0), std=(1,1,1). If a model ever returns a falsy/empty default, this would silently accept missing required preprocessing values and potentially proceed with an invalid input_size. Prefer raising a clear error when model defaults are unavailable, or restructure so only the new intensity fields default while required fields must come from either caller or model defaults.
| ("model_info", "storage_dtype"): dip.storage_dtype, | ||
| ("model_info", "intensity_mode"): dip.intensity_mode, | ||
| ("model_info", "percentile_low"): str(dip.percentile_low), | ||
| ("model_info", "percentile_high"): str(dip.percentile_high), | ||
| ("model_info", "scale_factor"): str(dip.scale_factor), | ||
| ("model_info", "min_value"): str(dip.min_value), | ||
| ("model_info", "repeat_channels"): str(dip.repeat_channels), | ||
| } | ||
| if dip.intensity_max_value is not None: | ||
| extra_data[("model_info", "intensity_max_value")] = str(dip.intensity_max_value) | ||
| if dip.window_center is not None: | ||
| extra_data[("model_info", "window_center")] = str(dip.window_center) | ||
| if dip.window_width is not None: | ||
| extra_data[("model_info", "window_width")] = str(dip.window_width) |
There was a problem hiding this comment.
_extend_model_metadata conditionally omits intensity_max_value/window_center/window_width when they are None. This conflicts with the PR description/issue goal of writing all intensity fields into exported rt_info, and means expected metadata keys may be missing for default configs (e.g. scale_to_unit with auto max_value). Consider always emitting these keys with a consistent sentinel value (e.g. empty string) so downstream consumers can rely on key presence.
| dip = self.data_input_params | ||
| extra_data = { | ||
| ("model_info", "mean_values"): mean_str.strip(), | ||
| ("model_info", "scale_values"): std_str.strip(), | ||
| ("model_info", "resize_type"): self.resize_mode, | ||
| ("model_info", "pad_value"): str(self.pad_value), | ||
| ("model_info", "reverse_input_channels"): str(self.swap_rgb), | ||
| ("model_info", "storage_dtype"): dip.storage_dtype, | ||
| ("model_info", "intensity_mode"): dip.intensity_mode, | ||
| ("model_info", "percentile_low"): str(dip.percentile_low), | ||
| ("model_info", "percentile_high"): str(dip.percentile_high), | ||
| ("model_info", "scale_factor"): str(dip.scale_factor), | ||
| ("model_info", "min_value"): str(dip.min_value), | ||
| ("model_info", "repeat_channels"): str(dip.repeat_channels), | ||
| } | ||
| if dip.intensity_max_value is not None: | ||
| extra_data[("model_info", "intensity_max_value")] = str(dip.intensity_max_value) | ||
| if dip.window_center is not None: | ||
| extra_data[("model_info", "window_center")] = str(dip.window_center) | ||
| if dip.window_width is not None: | ||
| extra_data[("model_info", "window_width")] = str(dip.window_width) |
There was a problem hiding this comment.
The new intensity-related metadata keys added in _extend_model_metadata are not covered by unit tests. Please add/extend exporter tests to assert the exported metadata contains the new keys (including the optional ones) and that their values match DataInputParams.
leoll2
left a comment
leoll2
left a comment
There was a problem hiding this comment.
Hi @omkar-334, thanks for the contribution. I would like to ask a few questions to verify your understanding of the issue:
- What does
intensity_moderepresent and why is it important to embed it in themodel_info? - Where are the intensity parameters meant to be consumed?
- Which other parameters should be non-null when
intensity_mode="scale_to_unit"? - Which model did you use for testing? Could you share the exported model as a .zip file?
| params["mean"] = self._datamodule.input_mean | ||
| if self._datamodule.input_std is not None: | ||
| params["std"] = self._datamodule.input_std | ||
| ic = getattr(self._datamodule, "intensity_config", None) |
There was a problem hiding this comment.
Why do you access the attribute via getattr here?
I used mobilenet_v3 model for testing. Here is the link for the exported model zip file - https://drive.google.com/file/d/15x1QL6blluC2Y5qNKJQAYHA4Vl3f8YXv/view?usp=sharing I used a script to check if the exported and reloaded model contains the saved metadata - from pathlib import Path
import openvino
import torch
from otx.backend.native.models.base import DataInputParams
from otx.backend.native.models.classification.multiclass_models.mobilenet_v3 import MobileNetV3MulticlassCls
#load model
model = MobileNetV3MulticlassCls(
model_name="mobilenetv3_large",
label_info=10,
data_input_params=DataInputParams(
input_size=(224, 224),
mean=(0.485, 0.456, 0.406),
std=(0.229, 0.224, 0.225),
storage_dtype="uint16",
intensity_mode="window",
intensity_max_value=65535.0,
window_center=40.0,
window_width=400.0,
),
)
model.eval()
output_dir = Path("exported_model")
output_dir.mkdir(exist_ok=True)
dummy_input = torch.randn(1, 3, 224, 224)
#export model to onnx
onnx_path = output_dir / "test_model.onnx"
torch.onnx.export(
model,
dummy_input,
str(onnx_path),
input_names=["input"],
output_names=["output"],
)
#convert to OpenVINO
ov_model = openvino.convert_model(str(onnx_path))
exporter = model._exporter
metadata = exporter.metadata
metadata = exporter._extend_model_metadata(metadata)
exporter._embed_openvino_ir_metadata(ov_model, metadata)
#save
xml_path = output_dir / "test_model.xml"
openvino.save_model(ov_model, str(xml_path))
#reload and verify metadata
ov_model = openvino.Core().read_model(str(xml_path))
print("\n=== Intensity metadata in exported IR ===")
for key in [
"storage_dtype", "intensity_mode", "intensity_max_value",
"window_center", "window_width", "percentile_low",
"percentile_high", "scale_factor", "min_value", "repeat_channels",
"mean_values", "scale_values",
]:
if ov_model.has_rt_info(["model_info", key]):
val = ov_model.get_rt_info(["model_info", key]).value
print(f" {key}: {val}")
else:
print(f" {key}: NOT FOUND")
print(f"\nModel saved to {output_dir.resolve()}")
print("Zip with: zip exported_model.zip exported_model/*")Output = |
there are 4 supported intensity modes -
They are consumed in the
|
|
hey @leoll2 the 2 failig tests are because of
|
|
Thanks for the PR, this enhancement was included in different PR |
Summary
library/src/otx/backend/native/models/base.py- Added fields toDataInputParams. Updatedas_dict()and the dict-merge in_configure_preprocessing_params.library/src/otx/backend/native/exporter/base.py-_extend_model_metadatanow writes all intensity fields into the exported IR.library/src/otx/data/module.py- Exposedintensity_configfromtrain_subset.intensityin both__init__andfrom_otx_datasets.library/src/otx/backend/native/engine.py- Readintensity_configfrom the datamodule and pass its fields into thedata_input_paramsdict.library/src/otx/cli/cli.py- Same as engine, for the CLI path.library/tests/unit/backend/native/models/test_base.py- Updatedtest_as_dictto match the new fields.Resolves #5952
How to test
Export a model and verify new metadata keys appear in the IR (storage_dtype, intensity_mode, etc.)
Checklist