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

New matrix generators #199

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

New matrix generators #199

wants to merge 6 commits into from

Conversation

tdehoff
Copy link

@tdehoff tdehoff commented Sep 27, 2024

No description provided.

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.

Looks good as a starting point. I added one commit to fix a few minor things. I'll merge this in for the release, and we can continue to develop it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine for now. I'm not sure if uplo, diag, and the intermediate test_matgen_dispatch routine are useful in this context, but we can address those later.

@@ -322,6 +322,8 @@ std::vector< testsweeper::routines_t > routines = {
{ "scale_row_col", test_scale_row_col, Section::aux },
{ "", nullptr, Section::newline },

{ "matgen", test_matgen, Section::aux_gen },
Copy link
Collaborator

@mgates3 mgates3 Oct 26, 2024

Choose a reason for hiding this comment

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

It probably makes more sense to move matgen to the end, after all the set routines.

Also, add a line:

    { "",                   nullptr,           Section::newline },

Check what ./tester -h prints out.

Hmm... on a second look, adding a newline may not make any difference, since it has its own section. I'll think about it.

//using real_t = blas::real_type<scalar_t>;
//using blas::real;
//using blas::imag;
//using slate::ceildiv;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I commented out some of these that are unused and the compiler might complain about. I also removed alpha and beta, which I don't think will be used for matgen.

GNUmakefile Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

I added matgen to the Makefile.

Currently, the Makefile explicitly lists all files, while CMake compiles all *.cc files. (BLAS++ and LAPACK++ are the opposite — Makefile compiles all *.cc files, while CMake explicitly lists them. Maybe someday we'll make that consistent.

@@ -669,6 +669,8 @@ def filter_csv( values, csv ):

[ 'scale_row_col', gen + dtype + mn + equed + nonuniform_nb + ge_matrix ],

[ 'matgen', gen_no_nb + dtype + ' --dim 20 --nb 4 --verbose 4' + ge_matrix ],

Copy link
Collaborator

Choose a reason for hiding this comment

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

I updated this so it tested only 20x20 matrix, nb 4. We need to revise run_tests.py, or use a different script, to save output to out and compare to ref.

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