-
Notifications
You must be signed in to change notification settings - Fork 8
Merge the miniapps as an optional #31
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
base: main
Are you sure you want to change the base?
Conversation
This facilitates contribution, keeping the repositories in sync, and exapnds from simple examples. Cleanup is necessary to make this more convenient for CI and more consistent. Co-authored-by: Max Melnichenko <m.y.melnich@gmail.com>
since we're using GTest, not CTest.
|
Hi @tcojean, are you ready to merge this (I see you've left it as "draft")? I think it's ready to merge, except for a couple of questions:
|
|
I suppose I have one more question, which is independent from functional testing, do we want to benchmark the MiniApps? (Benchmarking is a broader question, I suppose.) |
|
Including |
|
Hi Ben, thanks a lot for the cleanup. I think we could consider merging this. Like you say, the BLAS++ and LAPACK++ builds take a while, so I was not sure about activating CI just yet, at least until we have our own ressources. If the 2k Github CI limit is not a hard limit, we could consider activating the CI already. On whether they are required:
Note that BLAS++ and LAPACK++ also works on GPUs, so that could help pave the way for GPU miniapps, although the algorithms would need to be rewritten to rely on e.g. Tests and benchmarking:
|
This facilitates contribution, keeping the repositories in sync, and expands from simple examples.
Cleanup is necessary to make this more convenient for CI and more consistent.