-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
MatrixBandPart operation for TensorFlow Hub models #22447
MatrixBandPart operation for TensorFlow Hub models #22447
Conversation
build_jenkins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@himanshugupta11002, please update https://github.com/openvinotoolkit/openvino/blob/master/src/frontends/tensorflow/docs/supported_ops.md documentation. It is recently added new check. Without it, Ci will fail.
okay I'll update it |
Hi @himanshugupta11002, any update on the fix? It should be very easy:) Best regards, |
build_jenkins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please apply code formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please apply comment. code-formatting is not yet applied
build_jenkins |
can you please apply comments asap because we have code freeze for 2024.0 release tomorrow and your PR is highly recommended to be included? Best regards, |
build_jenkins |
make_shared<v1::Less>(make_shared<v1::Constant>(element::i64, Shape{}, {num_lower}), make_shared<v1::Constant>(element::i64, Shape{}, {0})), | ||
make_shared<v1::LessEqual>(make_shared<v1::Subtract>(m, n), make_shared<v1::Constant>(element::i64, Shape{}, {num_lower})) | ||
), | ||
make_shared<v1::LogicalOr>( | ||
make_shared<v1::Less>(make_shared<v1::Constant>(element::i64, Shape{}, {num_upper}), make_shared<v1::Constant>(element::i64, Shape{}, {0})), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix num_lower
and num_upper
usage here as well.
), | ||
make_shared<v1::LogicalOr>( | ||
make_shared<v1::Less>(make_shared<v1::Constant>(element::i64, Shape{}, {num_upper}), make_shared<v1::Constant>(element::i64, Shape{}, {0})), | ||
make_shared<v1::LessEqual>(make_shared<v1::Subtract>(n, m), make_shared<v1::Constant>(element::i64, Shape{}, {num_upper})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num_upper in Constant
Hi @himanshugupta11002, pls address comments above. I believe your PR is close to completeness. Best regards, |
Hi @rkazants, should I add comments in the matrixbandpart.cpp file, or should I add them in all files where I have made changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@himanshugupta11002, can you please revert this asap? That is because it is blocking to merge PR and to enable StableDiffusion model from Keras.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert this change
Hi @himanshugupta11002, I am closing this PR. I fix your implementation there: #23082. Your contribution is preserved there. Best regards, |
**Details:** `MatrixBandPart` is needed to support Keras StableDiffusion model. This is reserved PR for #22447 **Ticket:** CVS-133786 --------- Signed-off-by: Kazantsev, Roman <[email protected]> Co-authored-by: himanshugupta11002 <[email protected]>
Details:
Unsqueeze these ranges so that to have tensor of shapes [m, 1] and [1, n]
Problem when I am running on Codespaces it give me so many errors on which I am currently working on.
Tickets: