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

Extend Einsum Core and common transformation to support broadcasting, repeated labels and ellipsis #28151

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

mmikolajcz
Copy link
Contributor

@mmikolajcz mmikolajcz commented Dec 19, 2024

Details:

  • Extend core implementation to support extended broadcasting to comply with PyTorch op
  • Extend common decomposition to support broadcasting of same label, ellipsis and repeated labels
  • Repeated labels subgraph that doesn't depend on static shapes,
  • Support of ellipsis label that doesn't mark any dimension,
  • *Test cases"

Tickets:

Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: transformations OpenVINO Runtime library - Transformations labels Dec 19, 2024
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
@github-actions github-actions bot added the category: ONNX FE OpenVINO ONNX FrontEnd label Jan 8, 2025
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
@github-actions github-actions bot added the category: PyTorch FE OpenVINO PyTorch Frontend label Jan 8, 2025
…rom Einsum

Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
Signed-off-by: MATEUSZ MIKOLAJCZYK <MATEUSZ.MIKOLAJCZYK@intel.com>
@github-actions github-actions bot added the category: TEMPLATE OpenVINO Template plugin label Jan 21, 2025
…position

Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
…larity

Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
@mmikolajcz mmikolajcz changed the title [WIP] Extend Einsum Core and common transformation to support broadcasting, repeated labels and ellipsis Extend Einsum Core and common transformation to support broadcasting, repeated labels and ellipsis Jan 23, 2025
@mmikolajcz mmikolajcz marked this pull request as ready for review January 23, 2025 18:09
@mmikolajcz mmikolajcz requested review from a team as code owners January 23, 2025 18:09
@mmikolajcz mmikolajcz requested review from a team as code owners January 23, 2025 18:09
@mmikolajcz mmikolajcz requested review from mvafin and itikhono and removed request for a team January 23, 2025 18:09
…improved clarity

Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
Signed-off-by: Mateusz Mikolajczyk <mateusz.mikolajczyk@intel.com>
Copy link
Contributor

@mitruska mitruska left a comment

Choose a reason for hiding this comment

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

There are three model tests enabled in this PR, making it valuable work.
However, the complexity of the changes makes the review process non-trivial.
Please share more context about the problem that has been resolved, the solution applied to the decomposition, and the expected subgraph.

@@ -491,6 +837,7 @@ void contract_two_inputs(ov::pass::EinsumDecomposition* einsum_decompose_ptr,
std::vector<int64_t> common_labels_inds1, common_labels_inds2;
std::vector<int64_t> separate_labels_inds1, separate_labels_inds2;
std::vector<int64_t> reduced_labels_inds1, reduced_labels_inds2;
std::vector<std::string> common_labels, sep_labels1, sep_labels2, reduced_labels; // +++++
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::vector<std::string> common_labels, sep_labels1, sep_labels2, reduced_labels; // +++++
std::vector<std::string> common_labels, sep_labels1, sep_labels2, reduced_labels;

Comment on lines +1109 to +1110
!has_ellipsis_in_input || (input_nodes[i].get_partial_shape().rank().get_length() ==
static_cast<ov::Dimension::value_type>(labels.size() - 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

To call get_partial_shape().rank().get_length() static rank must be ensured, if it is ensured at this point, then get_partial_shape().size() could be used instead. Also it may be better to add to the input rank instead of subtract from the labels size, just to avoid overflow for empty labels vector.

Suggested change
!has_ellipsis_in_input || (input_nodes[i].get_partial_shape().rank().get_length() ==
static_cast<ov::Dimension::value_type>(labels.size() - 1));
!has_ellipsis_in_input || ((input_nodes[i].get_partial_shape().size() + 1) == labels.size());

@rkazants rkazants added this to the 2025.1 milestone Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph) category: ONNX FE OpenVINO ONNX FrontEnd category: PyTorch FE OpenVINO PyTorch Frontend category: TEMPLATE OpenVINO Template plugin category: transformations OpenVINO Runtime library - Transformations do not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants