Skip to content

Improved continuous integration testing#326

Merged
mhoemmen merged 10 commits intokokkos:mainfrom
NavalNuclearLab:improved-ci
Apr 30, 2026
Merged

Improved continuous integration testing#326
mhoemmen merged 10 commits intokokkos:mainfrom
NavalNuclearLab:improved-ci

Conversation

@amklinv-nnl
Copy link
Copy Markdown
Contributor

Now supports Windows/MSVC as well as Linux/gcc and MacOS/clang. Also tests for both 17 and 20 standard compatibility.

CMake may require Visual Studio to be listed as the generator, but that should be the default generator on Windows. Guess we'll find out!
$GITHUB_WORKSPACE does not work on Windows, so I replaced it with the hopefully OS agnostic ${{ github.workspace }}
The explicit paths were Linux-style, and Windows did not read them correctly.
@amklinv-nnl
Copy link
Copy Markdown
Contributor Author

The C++17 build is broken. I'll submit a separate PR fixing it.

@mhoemmen
Copy link
Copy Markdown
Contributor

mhoemmen commented Mar 18, 2026

Fix suggestions

proxy_reference.hpp

friend auto abs(const derived_type& x) {
  if constexpr (std::is_unsigned_v<value_type>) {
    return value_type(static_cast<const this_type&>(x));
  } else {
    return abs(value_type(static_cast<const this_type&>(x))); // causes build error, possibly because it fails to exclude `std::abs` via the `= delete` idiom
  }
}

This code probably predates abs-if-needed. We could try changing the implementation as follows.

friend auto abs(const derived_type& x) {
  return impl::abs_if_needed(value_type(static_cast<const this_type&>(x)));
}

amklinv-nnl added a commit to NavalNuclearLab/stdBLAS that referenced this pull request Apr 29, 2026
Thanks to @mhoemmen for the suggestion!
Thanks to @mhoemmen for the suggestion!
@amklinv-nnl
Copy link
Copy Markdown
Contributor Author

amklinv-nnl commented Apr 29, 2026

@mhoemmen I implemented the changes you suggested, and the build/tests are passing. Kindly consider accepting this PR at your convenience!

Copy link
Copy Markdown
Contributor

@mhoemmen mhoemmen left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes! : - )

@mhoemmen mhoemmen merged commit a3b21b0 into kokkos:main Apr 30, 2026
6 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