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

Add nonuniform tile sizes, device distributions, and grid orders to even more testers (depends on #143) #157

Merged
merged 33 commits into from
Feb 4, 2024

Conversation

neil-lindquist
Copy link
Contributor

@neil-lindquist neil-lindquist commented Dec 27, 2023

This PR continues the work of #143 to add support for nonuniform tile sizes, device distributions, and grid orders to more testers. I also made a couple of related warning messages more consistent. However, I didn't change the defaults of run-tests.py to test the added configurations.

The change is largely realized by refactoring the matrix creation logic into a helper function. This helps reduce the amount of duplicate code, which actually reduces the line count and the executable size a little bit.

Other improvements

  • Fixed a bug in henorm and synorm when tiles are nonuniform
  • Removed test/scalapack_support_routines.hh since it's no longer used
  • The meaning of the values of the dev-dist param was flipped to match slate/func.hh
  • The norm testers were adjusted to be more consistent between them.

This PR ended up longer than intended. I'd probably suggest starting with test/test_util.hh, test/matrix_util.hh, and test/matrix_util.cc. Also, the norm testers have some big indent changes, so I'd suggest reviewing that diff with "ignore whitespace" enabled.

Diff w/ #143

@mgates3
Copy link
Collaborator

mgates3 commented Jan 19, 2024

Since #143 is merged, can I drop the Draft label?
Locally I rebased this to include the changes that I added to #143. I need to double-check that code before I push it.

@neil-lindquist
Copy link
Contributor Author

Yes, the draft status was just waiting for #143.

@mgates3 mgates3 removed the Draft label Jan 23, 2024
@mgates3
Copy link
Collaborator

mgates3 commented Jan 23, 2024

I rebased this on top of the current master with the merged-in #143. There were some merge conflicts, which hopefully I managed correctly. (I did them twice and compared to make sure.)

Copy link
Collaborator

@mgates3 mgates3 left a comment

Choose a reason for hiding this comment

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

Some initial comments. More to come...

test/matrix_utils.hh Show resolved Hide resolved
src/internal/internal_henorm.cc Outdated Show resolved Hide resolved
src/internal/internal_synorm.cc Outdated Show resolved Hide resolved
src/internal/internal_synorm.cc Outdated Show resolved Hide resolved
src/internal/internal_synorm.cc Outdated Show resolved Hide resolved
test/run_tests.py Show resolved Hide resolved
src/internal/internal_synorm.cc Show resolved Hide resolved
Copy link
Collaborator

@mgates3 mgates3 left a comment

Choose a reason for hiding this comment

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

I'll see what I can fix. Some comments are for future work.

test/matrix_utils.cc Show resolved Hide resolved
test/matrix_utils.cc Show resolved Hide resolved
test/run_tests.py Outdated Show resolved Hide resolved
test/run_tests.py Show resolved Hide resolved
test/test.cc Outdated Show resolved Hide resolved
test/test_posv.cc Outdated Show resolved Hide resolved
test/test_symm.cc Show resolved Hide resolved
test/test_herk.cc Show resolved Hide resolved
test/test_herk.cc Outdated Show resolved Hide resolved
test/test_utils.hh Show resolved Hide resolved
@mgates3 mgates3 marked this pull request as ready for review January 31, 2024 05:50
@mgates3 mgates3 mentioned this pull request Jan 31, 2024
11 tasks
@mgates3 mgates3 merged commit bc574e9 into icl-utk-edu:master Feb 4, 2024
8 checks passed
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.

2 participants