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.

Sign up for GitHub

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

Jump to bottom

Simplify the testing types #1594

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Simplify the testing types #1594

wants to merge 5 commits into from

Conversation

thoasm
Copy link
Member

@thoasm thoasm commented Apr 8, 2024 •
edited

Add template-type functions to combine and merge type lists. As a result, the GINKGO_DPCPP_SINGLE_MODE only needs to be present once.

Additionally, change the typen name ValueAndIndexType to ComplexAndPODTypes (because gko::size_type is not an IndexType).

TODO:

  • Add documentation to the type helper
  • go through all existing tests and use the wrapper in all cases we define types on our own (example: CB-Gmres)

@ginkgo-bot ginkgo-bot added reg:build This is related to the build system. reg:testing This is related to testing. mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:reference This is related to the reference module. labels Apr 8, 2024
@thoasm thoasm force-pushed the simplify_test_types branch 2 times, most recently from d648b9a to 5d94d98 Compare April 11, 2024 17:02
@thoasm thoasm requested review from a team April 11, 2024 17:03
@thoasm thoasm self-assigned this Apr 11, 2024
@thoasm thoasm marked this pull request as ready for review April 11, 2024 17:04
@thoasm thoasm added 1:ST:ready-for-review This PR is ready for review 1:ST:no-changelog-entry Skip the wiki check for changelog update labels Apr 11, 2024
@MarcelKoch MarcelKoch self-requested a review April 19, 2024 07:50
MarcelKoch
MarcelKoch requested changes Apr 19, 2024
View reviewed changes
Copy link
Member

@MarcelKoch MarcelKoch left a comment

Choose a reason for hiding this comment

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

Generally lgtm. The biggest issue for me is the documentation. It's dupliated a lot and should only be in one place (I think I would prefer the aliases in the gko::test namespace).

core/test/utils.hpp Show resolved Hide resolved
core/test/utils.hpp Outdated Show resolved Hide resolved
core/test/utils/utils_test.cpp Show resolved Hide resolved
core/test/utils/utils_test.cpp Outdated Show resolved Hide resolved
core/test/utils/utils_test.cpp Show resolved Hide resolved
@MarcelKoch MarcelKoch added this to the Ginkgo 1.8.0 milestone Apr 19, 2024
@thoasm thoasm requested review from MarcelKoch and a team April 24, 2024 11:01
@MarcelKoch MarcelKoch removed this from the Ginkgo 1.8.0 milestone May 3, 2024
@tcojean tcojean added this to the Ginkgo 1.9.0 milestone May 3, 2024
Thomas Grützmacher added 4 commits July 11, 2024 15:22
@thoasm
Simplify the testing types
390fe7b 
Add template type functions to combine and merge type lists.
As a result, the GINKGO_DPCPP_SINGLE_MODE only needs to be present once.

Additionally, change the typen name ValueAndIndexType to ComplexAndPODTypes
(because gko::size_type is not an IndexType).
@thoasm
Use new type aliases in relevant tests
50bc383 
Additionally, introduce `IntegerTypes`, which is IndexTypes combined
with gko::size_type (used for the prefix sum tests).
@thoasm
Add documentation for the new type alias
94947dd
@thoasm
Update documentation for type list helper
9f45b22 
Additionally, add tests for empty inputs.
@thoasm thoasm force-pushed the simplify_test_types branch 2 times, most recently from e05756d to 869b2c4 Compare July 12, 2024 07:28
@ginkgo-bot
Copy link
Member

Error: The following files need to be formatted:

core/test/utils/utils_test.cpp

You can find a formatting patch under Artifacts here or run format! if you have write access to Ginkgo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarcelKoch MarcelKoch MarcelKoch approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@thoasm thoasm

Labels
1:ST:no-changelog-entry Skip the wiki check for changelog update 1:ST:ready-for-review This PR is ready for review mod:core This is related to the core module. mod:cuda This is related to the CUDA module. mod:reference This is related to the reference module. reg:build This is related to the build system. reg:testing This is related to testing.
Projects
None yet
Milestone
Ginkgo 1.9.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@thoasm @ginkgo-bot @MarcelKoch @tcojean