Skip to content

EAMxx: fix bug in physics unit tests baselines handling #7524

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

Merged
merged 3 commits into from
Jul 16, 2025
Merged

EAMxx: fix bug in physics unit tests baselines handling #7524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d8a47e
EAMxx: add possibility to use ASCII format for physics baselines files
bartgol Jul 16, 2025
7a8a9dd
EAMxx: fix bug in write_scalars function in physics baselines tests
bartgol Jul 16, 2025
5a34ad9
EAMxx: fix reading of p3_run_and_cmp baselines
bartgol Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions components/eamxx/src/physics/p3/tests/p3_run_and_cmp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,13 @@ struct Baseline {
EKAT_REQUIRE_MSG(dim == f.dim,
"For field " << f.name << " read expected dim " <<
f.dim << " but got " << dim);
std::vector<int> ds(dim);
impl::read_scalars(ifile,ds);
int extents[3];
impl::read_scalars(ifile,extents);
for (int i = 0; i < dim; ++i)
EKAT_REQUIRE_MSG(ds[i] == f.extent[i],
EKAT_REQUIRE_MSG(extents[i] == f.extent[i],
"For field " << f.name << " read expected dim "
<< i << " to have extent " << f.extent[i] << " but got "
<< ds[i]);
<< extents[i]);
impl::read_scalars(ifile,f.data, f.size);
// The code below is to force a result difference. This is used by the
// scream/scripts internal testing to verify that various DIFFs are detected.
Expand Down
19 changes: 18 additions & 1 deletion components/eamxx/src/physics/share/physics_test_read_write_helpers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,22 @@
#include <vector>
#include <type_traits>

#ifdef EAMXX_ASCII_BASELINES
#include <iomanip>
#include <limits>
#endif

namespace scream {
namespace impl {

template <typename T>
void read_scalars(std::ifstream& ifile, T& data)
{
#ifdef EAMXX_ASCII_BASELINES
ifile >> data;
#else
ifile.read(reinterpret_cast<char*>(&data),sizeof(data));
#endif
}

template <typename T>
Expand Down Expand Up @@ -47,7 +56,15 @@ void read_scalars(std::ifstream& ifile, T& data, S&... tail)
template <typename T>
void write_scalars(std::ofstream& ofile, const T& data)
{
#ifdef EAMXX_ASCII_BASELINES
if constexpr (std::is_floating_point_v<T>) {
ofile << std::fixed << std::setprecision(std::numeric_limits<T>::digits10+1) << data << " ";
} else {
ofile << data << " ";
}
#else
ofile.write(reinterpret_cast<const char*>(&data),sizeof(data));
#endif
}

template <typename T>
Expand All @@ -67,7 +84,7 @@ void write_scalars(std::ofstream& ofile, const T (&data)[N]) {

template <typename T, typename I>
std::enable_if_t<std::is_integral<I>::value>
write_scalars(std::ofstream& ofile, const T* const data, I N) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The const was preventing the compile to pick this overload for a T=double*. Instead, it picked the variadic template overload below, effectively writing a pointer and an int, rather than an array of T.

The const is also not needed, as the new signature covers it with T=const double.

write_scalars(std::ofstream& ofile, T* const data, I N) {
for (I i=0; i<N; ++i) {
write_scalars(ofile,data[i]);
}
Expand Down
8 changes: 8 additions & 0 deletions components/eamxx/src/share/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ target_compile_options(scream_share PUBLIC
if (SCREAM_CIME_BUILD)
target_compile_definitions (scream_share PUBLIC SCREAM_CIME_BUILD)
endif()

# When dealing with subtle bugs in read/write of unit tests baselines,
# using ascii baselines can help inspect that the file indeed looks as expected
option (EAMXX_ASCII_BASELINES "Whether physics units tests should assume ascii (instead of binary) format for baselines" OFF)
if (EAMXX_ASCII_BASELINES)
target_compile_definitions (scream_share PUBLIC EAMXX_ASCII_BASELINES)
endif()

# We have some issues with RDC and repeated libraries in the link line.
# It's a known issue, and nvcc_wrapper has a flag for handling this.
if (Kokkos_ENABLE_CUDA_RELOCATABLE_DEVICE_CODE)
Expand Down
Loading