Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saving external data for large ONNX models #255

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions optimum/onnxruntime/modeling_decoder.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@
from ..utils.save_utils import maybe_load_preprocessors, maybe_save_preprocessors
from .io_binding import TypeHelper
from .modeling_ort import ORTModel
from .utils import ONNX_DECODER_NAME, ONNX_DECODER_WITH_PAST_NAME, get_provider_for_device, parse_device
from .utils import (
ONNX_DECODER_NAME,
ONNX_DECODER_WITH_PAST_NAME,
_get_external_data_paths,
get_provider_for_device,
parse_device,
)


if TYPE_CHECKING:
Expand Down Expand Up @@ -470,14 +476,18 @@ def _save_pretrained(
The decoder with past key values model file name overwriting the default file name, allowing to save
the decoder model with a different name.
"""
src_paths = [self.decoder_model_path]
src_file_names = [self.decoder_model_path]
dst_file_names = [decoder_file_name]

if self.use_cache:
src_paths.append(self.decoder_with_past_model_path)
src_file_names.append(self.decoder_with_past_model_path)
dst_file_names.append(decoder_with_past_file_name)

for src_path, dst_file_name in zip(src_paths, dst_file_names):
dst_path = Path(save_directory).joinpath(dst_file_name)
# add external data paths in case of large models
src_file_names, dst_file_names = _get_external_data_paths(src_file_names, dst_file_names)

for src_path, dst_file_name in zip(src_file_names, dst_file_names):
dst_path = Path(save_directory) / dst_file_name
shutil.copyfile(src_path, dst_path)

@classmethod
Expand Down
13 changes: 10 additions & 3 deletions optimum/onnxruntime/modeling_ort.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
from .io_binding import IOBindingHelper, TypeHelper
from .utils import (
ONNX_WEIGHTS_NAME,
_get_external_data_paths,
get_device_for_provider,
get_provider_for_device,
parse_device,
Expand Down Expand Up @@ -300,9 +301,15 @@ def _save_pretrained(self, save_directory: Union[str, Path], file_name: str = ON
file_name (`str`, *optional*, defaults to the value of `optimum.onnxruntime.utils.ONNX_WEIGHTS_NAME`):
The filename to use when saving the model.
"""
# TODO: support models with external data
dst_path = Path(save_directory).joinpath(file_name)
shutil.copyfile(self.model_path, dst_path)
src_file_names = [self.model_path]
dst_file_names = [file_name]

# add external data paths in case of large models
src_file_names, dst_file_names = _get_external_data_paths(src_file_names, dst_file_names)

for src_path, dst_file_name in zip(src_file_names, dst_file_names):
dst_path = Path(save_directory) / dst_file_name
shutil.copyfile(src_path, dst_path)

@staticmethod
def _generate_regular_names_for_filename(filename: str):
Expand Down
4 changes: 4 additions & 0 deletions optimum/onnxruntime/modeling_seq2seq.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
ONNX_DECODER_NAME,
ONNX_DECODER_WITH_PAST_NAME,
ONNX_ENCODER_NAME,
_get_external_data_paths,
get_provider_for_device,
parse_device,
validate_provider_availability,
Expand Down Expand Up @@ -905,6 +906,9 @@ def _save_pretrained(
src_file_names.append(self.decoder_with_past_model_path)
dst_file_names.append(decoder_with_past_file_name)

# add external data paths in case of large models
src_file_names, dst_file_names = _get_external_data_paths(src_file_names, dst_file_names)

for src_path, dst_file_name in zip(src_file_names, dst_file_names):
dst_path = Path(save_directory) / dst_file_name
shutil.copyfile(src_path, dst_path)
Expand Down
20 changes: 20 additions & 0 deletions optimum/onnxruntime/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,23 @@ class ORTQuantizableOperator(Enum):
Resize = "Resize"
AveragePool = "AveragePool"
Concat = "Concat"


def _get_external_data_paths(src_file_names, dst_file_names):
import onnx
from onnx.external_data_helper import ExternalDataInfo, _get_initializer_tensors

model_paths = src_file_names.copy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a list of Paths so I don't think it copies anything here, we might as well start from a new empty list, and fill it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked without the copy() and the extending does modify the list src_file_names unfortunately :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my point is that you can create an empty list

model_paths = []

And fill it as you go?

My point here is that src_files_names[0] will be the same instance of model_paths[0].

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I don't quite get it. I only use model_paths to iterate over the inputted src_files_names. Then I keep on filling src_files_names

    model_paths = []
    for model_path in model_paths:
        # load model graph
        model = onnx.load(str(model_path), load_external_data=False)
        # filter out tensors that are not external data
        model_tensors = _get_initializer_tensors(model)
        model_tensors_ext = [
            ExternalDataInfo(tensor).location
            for tensor in model_tensors
            if tensor.HasField("data_location") and tensor.data_location == onnx.TensorProto.EXTERNAL
        ]
        src_paths.extend([model_path.parent / tensor_name for tensor_name in model_tensors_ext])
        dst_file_names.extend(model_tensors_ext)
    return src_paths, dst_file_names

So this shouldn't work

for model_path in model_paths:
# load model graph
model = onnx.load(str(model_path), load_external_data=False)
# filter out tensors that are not external data
model_tensors = _get_initializer_tensors(model)
model_tensors_ext = [
ExternalDataInfo(tensor).location
for tensor in model_tensors
if tensor.HasField("data_location") and tensor.data_location == onnx.TensorProto.EXTERNAL
]
src_file_names.extend([model_path.parent / tensor_name for tensor_name in model_tensors_ext])
dst_file_names.extend(model_tensors_ext)
return src_file_names, dst_file_names