openvino
openvino copied to clipboard
openvinotoolkit
[ONNX] Added support for Multinomial-7 operator
Details:
- Extend ONNX Frontend with Multinomial-7 operator
Tickets:
- 118986
I am Facing this Error:-
/home/pratham/Desktop/openvino/src/frontends/onnx/frontend/src/op/multinomial.cpp: In function ‘ov::OutputVector ov::frontend::onnx::op::set_1::multinomial(const ov::frontend::onnx::Node&)’:
/home/pratham/Desktop/openvino/src/frontends/onnx/frontend/src/op/multinomial.cpp:33:44: error: ‘class ov::descriptor::Tensor’ has no member named ‘data’
33 | auto data_ptr = input.get_tensor().data
@PRATHAM-SPS I've took a look on a docs.
https://github.com/onnx/onnx/blob/main/docs/Operators.md#Multinomial and https://docs.openvino.ai/2024/documentation/openvino-ir-format/operation-sets/operation-specs/generation/multinomial-13.html
In my understanding you don't need to generate samples at all. You should pass ONNX's "sample_size" to OpenVINO's "num_samples". @mitruska could you correct me if I'm wrong.
Also, important thing - you shouldn't ignore "seed" and "dtype". But because OpenVINO uses seed as a int64 - you should do it like here https://github.com/openvinotoolkit/openvino/blob/3d45a644969454d09c3f7e97ea60cbbd8abb0740/src/frontends/onnx/frontend/src/op/random_uniform.cpp#L28
And use this https://github.com/openvinotoolkit/openvino/blob/3d45a644969454d09c3f7e97ea60cbbd8abb0740/src/frontends/onnx/frontend/src/utils/common.cpp#L33 to convert dtype to ov::element::Type
Hello @PRATHAM-SPS, thank you for this contribution!
I agree with the @gkrivor previous comments. Added few more. The final solution should be similar to the RandomUniform.
Please confirm that you are working on resolving the comments.
Sure I will try to make necessary changes
@gkrivor and @mitruska , could you take a look at the code and tell me if I'm on the right track?
@PRATHAM-SPS do you have any updates here? seems we are about to cross the finish line with it
@PRATHAM-SPS do you have any updates here? seems we are about to cross the finish line with it
Hey @andrei-kochin , I'll get to it in 3 to 4 days. Pretty tied up with academic stuff, but I'll definitely catch up soon.
@PRATHAM-SPS do you have any updates here? seems we are about to cross the finish line with it
Hey @andrei-kochin , I'll get to it in 3 to 4 days. Pretty tied up with academic stuff, but I'll definitely catch up soon.
@PRATHAM-SPS Good luck!
@PRATHAM-SPS any news for us?
I've made some slight adjustments based on feedback, and now I'm testing it out on my end.
@PRATHAM-SPS I suggest to modify tests and verify only output shapes. But you need to make input parameters slightly more complicated because: Output tensor with shape [batch_size, sample_size] So, you need to provide batch_size > 1 and sample_size > 1 (I think something around 5-10 is ok) to get some checks. We cannot just verify an output is only [1,1]. Do you need any help with tests?
@PRATHAM-SPS I suggest to modify tests and verify only output shapes. But you need to make input parameters slightly more complicated because: Output tensor with shape [batch_size, sample_size] So, you need to provide batch_size > 1 and sample_size > 1 (I think something around 5-10 is ok) to get some checks. We cannot just verify an output is only [1,1]. Do you need any help with tests?
I think there is some build issue on my system, all other test cases are getting failed. I have tried with a mish 18 operator, it is also getting failed at my end. And surely, I will need some assistance from you to cover multinomial test cases as its outcome is probable. Thank you for your assistance.