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

#5635: Serial/OpenMP: Parallel_scan with return value for TeamThreadRange #6090

Merged

Conversation

thearusable
Copy link
Contributor

Related to #5635

@thearusable thearusable self-assigned this Apr 28, 2023
@thearusable thearusable changed the title #5635: Serial: Add Overloads for parallel_scan with return value [WIP] #5635: Serial: Add Overloads for parallel_scan with return value Apr 28, 2023
@crtrott crtrott linked an issue May 3, 2023 that may be closed by this pull request
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch 3 times, most recently from 1fec60a to 51f46c5 Compare May 8, 2023 13:20
Copy link
Contributor

@fnrizzi fnrizzi left a comment

Choose a reason for hiding this comment

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

see comments

@fnrizzi
Copy link
Contributor

fnrizzi commented May 9, 2023

please improve description

fnrizzi

This comment was marked as duplicate.

@fnrizzi
Copy link
Contributor

fnrizzi commented May 9, 2023

i think we are missing some guards in the test, aren't we? how about openmp and threads?
https://kokkos.github.io/kokkos-core-wiki/keywords.html#device-backends

@fnrizzi fnrizzi added this to the Release 4.1 milestone May 9, 2023
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from 51f46c5 to 082ed05 Compare May 10, 2023 10:06
@thearusable
Copy link
Contributor Author

@fnrizzi Yes I missed some guards, now they are added. Also I improved the comments in tests.

@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch 4 times, most recently from a7358ca to 6f66c24 Compare May 12, 2023 14:55
@thearusable thearusable changed the title [WIP] #5635: Serial: Add Overloads for parallel_scan with return value [WIP] #5635: Serial/OpenMP: Add Overloads for parallel_scan with return value May 12, 2023
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from 6f66c24 to 1cf709c Compare May 18, 2023 09:19
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from 1cf709c to 7317501 Compare July 6, 2023 12:49
@thearusable thearusable marked this pull request as ready for review July 6, 2023 13:00
@thearusable thearusable changed the title [WIP] #5635: Serial/OpenMP: Add Overloads for parallel_scan with return value #5635: Serial/OpenMP: Add Overloads for parallel_scan with return value Jul 6, 2023
@thearusable thearusable requested a review from fnrizzi July 6, 2023 13:00
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from 7317501 to 91c8633 Compare July 10, 2023 10:10
@thearusable thearusable changed the title #5635: Serial/OpenMP: Add Overloads for parallel_scan with return value #5635: [1/X] Serial/OpenMP: Add Overloads for parallel_scan with return value Jul 10, 2023
core/src/impl/Kokkos_HostThreadTeam.hpp Outdated Show resolved Hide resolved
core/src/impl/Kokkos_HostThreadTeam.hpp Outdated Show resolved Hide resolved
core/unit_test/incremental/Test16_ParallelScan.hpp Outdated Show resolved Hide resolved
@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from 74a155a to 56cc56d Compare July 18, 2023 14:16
@fnrizzi
Copy link
Contributor

fnrizzi commented Jul 24, 2023

retest this please

@fnrizzi
Copy link
Contributor

fnrizzi commented Jul 24, 2023

@thearusable please make sure you also post a draft PR in the wiki to update the documentation on the parallel_scan interface to reflect this PR

Copy link
Contributor

@fnrizzi fnrizzi left a comment

Choose a reason for hiding this comment

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

see comment

core/unit_test/incremental/Test16_ParallelScan.hpp Outdated Show resolved Hide resolved
@fnrizzi
Copy link
Contributor

fnrizzi commented Jul 27, 2023

retest this please

@fnrizzi
Copy link
Contributor

fnrizzi commented Jul 27, 2023

@thearusable this still have failures

for example:

4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 495, which exceeds abs_err, where
4: scan_ref evaluates to 561,
4: a_os(i) evaluates to 66, and
4: abs_err evaluates to 4.0531158447265625e-06.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 1521, where
4: scan_ref evaluates to 1711,
4: a_os(i) evaluates to 190.
4: The abs_error parameter abs_err evaluates to 1.3100631690576847e-14 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 2.8421709430404007e-14, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 1849, which exceeds abs_err, where
4: scan_ref evaluates to 2080,
4: a_os(i) evaluates to 231, and
4: abs_err evaluates to 7.7486038208007812e-06.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 61009, where
4: scan_ref evaluates to 68635,
4: a_os(i) evaluates to 7626.
4: The abs_error parameter abs_err evaluates to 8.2378548427186615e-14 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 9.0949470177292824e-13, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:238: Failure
4: Expected equality of these values:
4:   scan_ref
4:     Which is: 486591
4:   a_os(i)
4:     Which is: 53956
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 763439, where
4: scan_ref evaluates to 858705,
4: a_os(i) evaluates to 95266.
4: The abs_error parameter abs_err evaluates to 2.9110047705671604e-13 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 1.4551915228366852e-11, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: [  FAILED  ] openmp.team_scan_ret_val (1001 ms)

@masterleinad
Copy link
Contributor

Please rebase on top of develop to fix some CI issues.

@thearusable thearusable force-pushed the 5635-serial-parallel-scan-with-value-2 branch from e0db953 to 856e22f Compare July 28, 2023 14:00
@thearusable
Copy link
Contributor Author

@thearusable this still have failures

for example:

4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 495, which exceeds abs_err, where
4: scan_ref evaluates to 561,
4: a_os(i) evaluates to 66, and
4: abs_err evaluates to 4.0531158447265625e-06.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 1521, where
4: scan_ref evaluates to 1711,
4: a_os(i) evaluates to 190.
4: The abs_error parameter abs_err evaluates to 1.3100631690576847e-14 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 2.8421709430404007e-14, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 1849, which exceeds abs_err, where
4: scan_ref evaluates to 2080,
4: a_os(i) evaluates to 231, and
4: abs_err evaluates to 7.7486038208007812e-06.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 61009, where
4: scan_ref evaluates to 68635,
4: a_os(i) evaluates to 7626.
4: The abs_error parameter abs_err evaluates to 8.2378548427186615e-14 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 9.0949470177292824e-13, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:238: Failure
4: Expected equality of these values:
4:   scan_ref
4:     Which is: 486591
4:   a_os(i)
4:     Which is: 53956
4: /var/jenkins/workspace/Kokkos/core/unit_test/TestTeamScan.hpp:240: Failure
4: The difference between scan_ref and a_os(i) is 763439, where
4: scan_ref evaluates to 858705,
4: a_os(i) evaluates to 95266.
4: The abs_error parameter abs_err evaluates to 2.9110047705671604e-13 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 1.4551915228366852e-11, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
4: [  FAILED  ] openmp.team_scan_ret_val (1001 ms)

Ok, fixed

@thearusable
Copy link
Contributor Author

Please rebase on top of develop to fix some CI issues.

Ok, rebased

@crtrott
Copy link
Member

crtrott commented Jul 31, 2023

I am a bit concerned about NVHPC not having run, but will risk merging it anyway.

@crtrott crtrott merged commit 929cac2 into kokkos:develop Jul 31, 2023
27 of 28 checks passed
@masterleinad masterleinad mentioned this pull request Sep 14, 2023
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

5 participants