[OV20] Layout class implementation - basic API #7452
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed setters - Removed LayoutRank from public classes
ilyachur
reviewed
Sep 13, 2021
| int64_t m_right_size = 0; | ||
| }; | ||
|
|
||
| namespace layouts { |
ngraph/test/layout.cpp
Outdated
| TEST(layout, custom_dims) { | ||
| Layout l = "0ac"; | ||
| EXPECT_FALSE(layouts::has_batch(l)); | ||
| EXPECT_ANY_THROW(layouts::batch(l)); |
There was a problem hiding this comment.
Ok, will correct. Also will change exception type from ngraph_error to ov::AssertFailure
- Rename 'layouts' namespace to 'layout' - 'get_index_by_name' - specify throw exception type
ilya-lavrenov
approved these changes
Sep 13, 2021
| static const std::map<std::string, std::string>& dim_aliases() { | ||
| static const std::map<std::string, std::string> DIM_ALIASES = {{BATCH, BATCH}, | ||
| {"BATCH", BATCH}, | ||
| {"B", BATCH}, |
There was a problem hiding this comment.
do we really need "B" as batch? I don't think it's a common name for batch
@jane-intel do you have any advices here?
| auto layout = std::dynamic_pointer_cast<VariantWrapper<Layout>>(it->second); | ||
| OPENVINO_ASSERT(layout, "Layout runtime info for node is invalid"); | ||
| if (!layout::has_channels(layout->get())) { | ||
| return -1; |
There was a problem hiding this comment.
-1 means channels is last. Maybe we need different invalid value?
|
|
||
| static StaticShape construct_mean_scale_shape(const std::shared_ptr<Node>& node, size_t values_size) { | ||
| // TODO: support also Mean/Scale image case | ||
| auto channels = get_channels_helper(node); |
There was a problem hiding this comment.
channels_index? Channels here seems to be a number of channels itself..
|
Let's fix comments in next PR together with case when we add mean / scale preprocessing for case |
akuporos
pushed a commit
to akuporos/openvino
that referenced
this pull request
Sep 29, 2021
* Draft * More tests * to_string + advanced_syntax + more tests * Coding style * Add mean/scale - vector version with layout support Vector version requires layout to be set * Added comments to LayoutRank * Removed unnecessary public API - Removed setters - Removed LayoutRank from public classes * Review comments: - Rename 'layouts' namespace to 'layout' - 'get_index_by_name' - specify throw exception type
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details:
Tickets: