onnx model with external data

[riccardopinosio]

Currently session creation fails for onnx models with external data (i.e. any model larger than 2GB) in linux-x64.

For instance, trying to get input/output info for the following model: https://huggingface.co/microsoft/Phi-3-mini-4k-instruct-onnx/tree/main/cpu_and_mobile/cpu-int4-rtn-block-32 when saved inside a subfolder:

SetSharedLibraryPath("/lib64/onnxruntime.so")
err := InitializeEnvironment()
if err != nil {
    t.Fatal(err)
}
defer DestroyEnvironment()
inputs, _, err := GetInputOutputInfo("Microsoft_Phi-3-mini-4k-instruct-onnx/phi3-mini-4k-instruct-cpu-int4-rtn-block-32.onnx")
if err != nil {
    t.Fatal(err)
}

One gets:

--- FAIL: TestLoadExternal (0.05s)
onnxruntime_test.go:1524: Error getting inputs and outputs from Microsoft_Phi-3-mini-4k-instruct-onnx/phi3-mini-4k-instruct-cpu-int4-rtn-block-32.onnx: Error creating temporary session: Deserialize tensor model.layers.32.final_norm_layernorm.weight failed.GetFileLength for ./phi3-mini-4k-instruct-cpu-int4-rtn-block-32.onnx.data failed:Invalid fd was supplied: -1
FAIL

I could only make this work by ensuring that the .data file is in the current working directory, as onnx by default seems to check from there based on the harcoded relative path in the onnx file.

It would be nice if this was handled by onnxruntime_go however. The only idea I have at the moment is to temporarily change the cwd to the directory containing the onnx file when creating a session so that onnx can find the data file.

[yalue]

Thanks for raising this issue, as I have never tried to load a file with external data. Unfortunately, even the C API for supplying external data is very clunky to use (see AddExternalInitializers or AddExternalInitializersFromFilesInMemory in onnxruntime_c_api.h), so I doubt there will be a "clean" approach in Go. I'm actually fine with temporarily setting the cwd when creating the session. If you or someone else wants to work on a change for this, I'd be happy to take a look at it.

At a quick glance, this is what I would suggest:

1. Modify the go SessionOptions struct to have a field such as workingDirDuringLoad or something like that (there's got to be a better name).
2. Add a setter function to SessionOptions, such as SetTemporaryCWDForLoading that sets the field.
3. Modify the createCSession function to something like this:

   // ...
   var ortSession *C.OrtSession
   var ortSessionOptions *C.OrtSessionOptions
   var tempWorkingDir string
   if options != nil {
       ortSessionOptions = options.o
       tempWorkingDir = options.workingDirDuringLoad
   }
   if tempWorkingDir != "" {
       prevDir, e := os.Getwd()
       if e != nil {
           return nil, fmt.Errorf("Error getting previous working dir: %w", e)
       }
       e = os.Chdir(tempWorkingDir)
       if e != nil {
           return nil, fmt.Errorf("Error setting temp working dir: %w", e)
       }
       defer os.Chdir(prevDir)
   }
   status := C.CreateSession(unsafe.Pointer(&(onnxData[0])),
       C.size_t(len(onnxData)), ortEnv, &ortSession, ortSessionOptions)
   // ...

   4. Somehow modify the GetInputOutputInfo and GetModelMetadata functions to pass the directory containing the .onnx file down the stack to createCSession. This will be a bit annoying, because I don't want to change the signature for the ...WithONNXData variants of these functions. (Similarly, I don't want to call the C API functions that take a path due to the windows compatibility headache. Maybe it's time to just figure out a clean way to convert UTF-8 to UTF-16 strings when using the windows APIs...)

[riccardopinosio]

hi, it seems a bit diffcult to pass down the .onnx directory to the withOnnxData functions without modifying their signature. The only solution I can think of is to add functions like:

...WithOnnxDataAndOptions(data []byte, options *SessionOptions)

which essentially duplicate the withOnnxData function but with options. Then the GetInputOutputInfo can switch to this function internally, but the withOnnxData functions remain intact. Not the most elegant solution but the only one I can see other than implementing support for the UTF paths C api stuff.

[yalue]

Does this problem still persist after commit b5a29a1390f985830891 (PR #69)? After that commit, the CreateSession APIs should be taking file paths directly rather than using file buffers, so I'm hoping that allows onnxruntime to find external data more easily.

[riccardopinosio]

great stuff will check later