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

[Tests] Shorten sycl_iterator and permutation_iterator tests #1552

Merged
merged 6 commits into from
May 23, 2024

Conversation

danhoeflinger
Copy link
Contributor

Adds infrastructure to be able to shorten some commonly used test infrastructure by

  1. allowing scaling of the max_n (100000) and small_n (16)
  2. allowing scaling of the step (3.14.15) multiplier

This also corrects a potential error in the search_20 test of sycl_iterator_find which for some value of n, could return the incorrect result due to a unintentional extension of the searched for list by the nature of the input data.

A few tests were scaled in this PR to have fewer iterations with different sizes:

sycl_iterator_find
sycl_iterator_reduce
sycl_iterator_scan
sycl_iterator_sort
permutation_iterator_parallel_merge
permutation_iterator_parallel_stable_sort

Others were just given the opportunity for similar shortening.

These patterns should also be tested via their individual tests, independently from the sycl_iterator type or the permutation_iterator types. Also, the newly added input_data_sweep tests, provide much more coverage for input data processing to dpcpp backends.

Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
@danhoeflinger
Copy link
Contributor Author

Note: This does not do anything to address time spent in JIT.
Its quite possible that a lot of the time of the test is spent in JIT compilation, and only removing kernel invocations or splitting the test can address this.

@SergeyKopienko
Copy link
Contributor

Should we think about ability to save previous behavior when the macros TEST_LONG_RUN is defined?

@danhoeflinger
Copy link
Contributor Author

Should we think about ability to save previous behavior when the macros TEST_LONG_RUN is defined?

Yes, perhaps with a we can do a min / max with the scale factor of 1.0 when TEST_LONG_RUN is defined so that we don't shorten a test someone intentionally made longer with the scale factors. Its a good idea.

Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
Signed-off-by: Dan Hoeflinger <dan.hoeflinger@intel.com>
@danhoeflinger
Copy link
Contributor Author

danhoeflinger commented May 9, 2024

Should we think about ability to save previous behavior when the macros TEST_LONG_RUN is defined?

I've now made it so when TEST_LONG_RUN is true, we undo any reduction in test cases from this. We allow scaling factors to still increase the number of tests in this case. None of the current code uses these scaling factors to increase the number of tests, but it is possible in the future, and we wouldn't want to reduce that for TEST_LONG_RUN.

@danhoeflinger
Copy link
Contributor Author

Actually... it looks like TEST_LONG_RUN=1 is used in the nightly testing, so it kind of defeats the purpose of this PR to turn off the savings in this case if our goal is to speed up the full suite testing.

@danhoeflinger
Copy link
Contributor Author

Actually... it looks like TEST_LONG_RUN=1 is used in the nightly testing, so it kind of defeats the purpose of this PR to turn off the savings in this case if our goal is to speed up the full suite testing.

I'm tempted to undo my TEST_LONG_RUN changes so we can take advantage of the savings in the nightly suite.

@timmiesmith @SergeyKopienko What are your thoughts?

@danhoeflinger
Copy link
Contributor Author

I'm tempted to undo my TEST_LONG_RUN changes so we can take advantage of the savings in the nightly suite.

I've reverted this change, because the point is to provide savings to the nightly suite. We can examine this further, perhaps we want some TEST_VERY_LONG_RUN option or something.

@SergeyKopienko SergeyKopienko self-requested a review May 23, 2024 13:28
Copy link
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

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

LGTM

@danhoeflinger danhoeflinger merged commit a0cb90a into main May 23, 2024
20 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/shorten_sycl_iterator_find branch May 23, 2024 13:34
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.

None yet

2 participants