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.

Sign up for GitHub

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

Jump to bottom

[optimizer] Allow optimizer to function when external data is not available #1792

Open
justinchuby opened this issue Aug 8, 2024 · 4 comments
Open

[optimizer] Allow optimizer to function when external data is not available #1792

justinchuby opened this issue Aug 8, 2024 · 4 comments
Labels
enhancement New feature or request topic: optimizer

Comments

@justinchuby
Copy link
Collaborator

Models with external data may not have their data available when being passed into the optimizer. We should still optimize what we can without raising an error.
@justinchuby justinchuby added enhancement New feature or request topic: optimizer labels Aug 8, 2024
@justinchuby justinchuby changed the title Allow optimizer to function when external data is not available [optimizer] Allow optimizer to function when external data is not available Aug 8, 2024
@justinchuby justinchuby mentioned this issue Aug 13, 2024
Merged
justinchuby added a commit that referenced this issue Aug 13, 2024
@justinchuby
[IR] Implement save/load functions in IR and handle external data pro…
87d7c4f 
…perly (#1801)

Implement efficient save/load and handle loading external data properly
in the IR.

Before this change, when a ModelProto containing external data is
converted to IR, the external tensor objects will load the data from a
path relative to the working directory, not the ONNX file. This is
because we do not store the onnx file path and thus have no way to look
for the external data file.

With the change, a `base_dir` property is added to ExternalTensor that
we can set, in a separate pass when the directory is available, so the
object has full information to find the data file on disk. The base_dir
is not serialized to the proto to maintain a relative path in the
"location" field in TensorProto.

#1701,
#1792

Example:

```
>>> m.graph.initializers["model.model.decoder.layers.2.encoder_attn.v_proj.weight"].const_value.display()
ExternalTensor<FLOAT,[512,512]>(path='model.onnx.data', 
name='model.model.decoder.layers.2.encoder_attn.v_proj.weight', offset=245864448, length=1048576, 
base_dir='/home/justinchu/dev/ONNXConverter/docker/dump_bash_bench/BlenderbotSmallForConditionalGeneration-torch
-onnx-detailed-cpu-')

Min: -0.08586505800485611, Max: 0.09103105217218399, NaN count: 0, Inf count: 0
Sparsity (abs<1e-06): 0.00
Histogram:
   11504 ┼
   10226 ┤                                  ╭───────╮
    8948 ┤                                ╭─╯       ╰─╮
    7670 ┤                              ╭─╯           ╰─╮
    6392 ┤                            ╭─╯               ╰─╮
    5113 ┤                          ╭─╯                   ╰─╮
    3835 ┤                        ╭─╯                       ╰─╮
    2557 ┤                     ╭──╯                           ╰─╮
    1279 ┤                ╭────╯                                ╰────╮
       1 ┼────────────────╯                                          ╰───────────────────
    -0.0859  -0.0682  -0.0505  -0.0306  -0.0129  0.0070  0.0225  0.0402  0.0557  0.0733  0.0910
```
@justinchuby
Copy link
Collaborator Author

cc @gramalingam

@gramalingam
Copy link
Collaborator

I took a quick look at the IR-based constant-folder/optimizer. I think it should work, as long as the numpy() method returns None or value.const_value is None. We can change it to use whatever appropriate methods the IR exposes to access such external tensors, allowing for possibility that the value may not be available.

On a different note: I just remembered that ir.Value.const_value is used for initializers that are graph inputs as well. This could break a few things. The optimizer etc. assume .const_value has a value only for constants, but graph inputs that are initializers should NOT be treated as constants.

@gramalingam
Copy link
Collaborator

Is this manifesting itself in any benchmark run (or another model)? Perhaps for proto-based optimization? We should deprecate that anyway.

@justinchuby
Copy link
Collaborator Author

I took a quick look at the IR-based constant-folder/optimizer. I think it should work, as long as the numpy() method returns None or value.const_value is None. We can change it to use whatever appropriate methods the IR exposes to access such external tensors, allowing for possibility that the value may not be available.

On a different note: I just remembered that ir.Value.const_value is used for initializers that are graph inputs as well. This could break a few things. The optimizer etc. assume .const_value has a value only for constants, but graph inputs that are initializers should NOT be treated as constants.

Right. We shouldn’t assume only Constants have const_value. Any value can have constant values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request topic: optimizer
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gramalingam @justinchuby