Skip to content

[compute/cker] Reduce duplicated code and Fix wrong shape size.#15158

Merged
chunseoklee merged 2 commits intoSamsung:masterfrom
ragmani:cker/shape_reduce_dumpliation
Apr 14, 2025
Merged

[compute/cker] Reduce duplicated code and Fix wrong shape size.#15158
chunseoklee merged 2 commits intoSamsung:masterfrom
ragmani:cker/shape_reduce_dumpliation

Conversation

@ragmani
Copy link
Copy Markdown
Contributor

@ragmani ragmani commented Apr 14, 2025

This commit refactors dims storage initialization of Shape.

ONE-DCO-1.0-Signed-off-by: ragmani ragmani0216@gmail.com

@ragmani ragmani added the PR/ready for review It is ready to review. Please review it. label Apr 14, 2025
@ragmani ragmani requested a review from a team April 14, 2025 03:12
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 14, 2025

This PR reduces duplicated code and fixes a test error(#15030 (comment)) introduced in #15155.

@ragmani ragmani force-pushed the cker/shape_reduce_dumpliation branch from c3779bd to 0697d3f Compare April 14, 2025 03:18
This commit refactors dims storage initialization of Shape.
  - Reduce duplicated code
  - Fix wrong shape size

ONE-DCO-1.0-Signed-off-by: ragmani <ragmani0216@gmail.com>
@ragmani ragmani force-pushed the cker/shape_reduce_dumpliation branch from 0697d3f to a99b62a Compare April 14, 2025 04:39
@ragmani ragmani changed the title [compute/cker] Reduce duplicated code. [compute/cker] Reduce duplicated code and Fix wrong shape. Apr 14, 2025
@ragmani ragmani changed the title [compute/cker] Reduce duplicated code and Fix wrong shape. [compute/cker] Reduce duplicated code and Fix wrong shape size. Apr 14, 2025
Comment thread runtime/compute/cker/include/cker/Shape.h Outdated
@ragmani ragmani added PR/ready for review It is ready to review. Please review it. and removed PR/ready for review It is ready to review. Please review it. labels Apr 14, 2025
@ragmani ragmani force-pushed the cker/shape_reduce_dumpliation branch 2 times, most recently from ee15559 to 447c678 Compare April 14, 2025 06:45
{
::testing::InitGoogleTest(&argc, argv);

::testing::GTEST_FLAG(death_test_style) = "threadsafe";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I set ::testing::GTEST_FLAG(death_test_style) = "threadsafe"; to ensure that death tests are executed in a thread-safe manner. This forces GoogleTest to spawn a new process using exec() instead of using fork(), which is safer when multiple threads are running. "Additionally, with this flag, GoogleTest no longer generates the following warning message."

[WARNING] /working_directory/ONE/externals/GTEST/googletest/src/gtest-death-test.cc:1108:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 17 threads. See https://github.com/google/googletest/blob/master/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out

You can find more details in the offical documentation:

@ragmani ragmani removed the PR/ready for review It is ready to review. Please review it. label Apr 14, 2025
…ializations.

- Add Shape unittests.

ONE-DCO-1.0-Signed-off-by: ragmani <ragmani0216@gmail.com>

// Constructor that creates a shape of given size and fills all dimensions with "value".
Shape(int shape_size, int32_t value) : _size(0)
Shape(int shape_size, int32_t value) : _size(shape_size)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This generated test errors because this constructor did not apply shape_size to _size anywhere.

@ragmani ragmani force-pushed the cker/shape_reduce_dumpliation branch from 447c678 to ea488a8 Compare April 14, 2025 08:05
@ragmani
Copy link
Copy Markdown
Contributor Author

ragmani commented Apr 14, 2025

I checked that tests pass on my local machine.

$ Product/x86_64-linux.debug/out/unittest/nnfw_api_gtest
...
[==========] 685 tests from 36 test suites ran. (811 ms total)
[  PASSED  ] 685 tests.
$ Product/x86_64-linux.debug/out/unittest/test_cker
...
[==========] 83 tests from 4 test suites ran. (1760 ms total)
[  PASSED  ] 83 tests.

Copy link
Copy Markdown
Contributor

@ys44kim ys44kim left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chunseoklee chunseoklee merged commit 97d6d49 into Samsung:master Apr 14, 2025
10 checks passed
@ragmani ragmani deleted the cker/shape_reduce_dumpliation branch April 14, 2025 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants