ONNX Engine Options Bug, ONNX features cannot be defined，It's a parameter type design problem

[TommysLee]

trafficstars

Description

Generic parameters are inconsistent problem, fail to deliver correctly. Details code is as follows:

code in ai.djl.repository.zoo.Criteria

public class Criteria<I, O> {
    ...
    private Map<String, String> options;
    private Map<String, Object> arguments;
    ....
        
    // line 635    
    public Builder<I, O> optOptions(Map<String, String> options) {
        this.options = options;
        return this;
    }
    
    // line 647
    public Builder<I, O> optOption(String key, String value) {
        if (options == null) {
            options = new HashMap<>();
        }
        options.put(key, value);
        return this;
    }
    
    ...
}

Here, optOptions() method, the parameter is Map<String, String> or both key and value are String

code in ai.djl.onnxruntime.engine.OrtModel

public class OrtModel extends BaseModel {
    ...
    
    // line 149
    private SessionOptions getSessionOptions(Map<String, ?> options) throws OrtException {
        SessionOptions ortSession = sessionOptions;
        if (options.containsKey("sessionOptions")) {
            ortSession = (SessionOptions) options.get("sessionOptions"); // here important
        }
    }
    
    ...
}

Since only pass the String data type in Criteria, an object of type SessionOptions here can never be passed here, and a custom SessionOptions object can never be obtained. In turn, many of ONNX's features cannot be released.

A String object cannot be converted to a SessionOptions object. This bug too obvious. Do you agree?

Suggestion

In order to solve this problem, the suggestions are as follows:

• Option 1

  adjust this：private Map<String, String> options

  Put no limit on value.

  I haven't looked at it as a whole, and I don't know if there are other problems.

• Option 2

  reuse this: private Map<String, Object> arguments

  This is a small change, we only need to add another layer of decision to the getSessionOptions() function

  Again, I didn't consider the whole picture, whether this change is appropriate.

• Option 3

  Redesign the delivery mechanism for ONNX Engine, but this may be more cumbersome.

  I don't have any good suggestions for this plan.

Environment Info

OS: Windows 10
DJL: 0.27.0
ONNX Runtime: 1.17.1
JDK: 17.0

[zachgk]

For now, you can directly create a model with Model.newInstance(..) and then call model.load(..) without going through the criteria

[frankfliu]

@TommysLee

Most of SessionOptions can be configured by individual key already, See: https://github.com/deepjavalibrary/djl/blob/master/engines/onnxruntime/onnxruntime-engine/src/test/java/ai/djl/onnxruntime/engine/OrtTest.java#L76-L83

We didn't expose all options and we didn't see demand for those missing options. As Zach suggested, for those uncommon options, you have to use OrtModel directly.

Let us know if you think any of of the missing option are important.

[TommysLee]

The point is "How to pass the sessionOptions parameter by Criteria.builder()"

[TommysLee]

From the source code perspective, it supports the sessionOptions parameter, but the type is incorrect because it is a String.