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.

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

eospac serialization fix #417

Merged
merged 6 commits into from
Sep 13, 2024
Merged

eospac serialization fix #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
45b2d31
changes needed to build against newest hdf5 in spack, add test for eo…
jonahm-LANL Sep 11, 2024
15b130e
formatting
jonahm-LANL Sep 11, 2024
7162b31
fix typo
jonahm-LANL Sep 11, 2024
0665f16
Merge branch 'jmm/eospac-serialization-fix' of github.com:lanl/singul…
jonahm-LANL Sep 11, 2024
97f7537
CHANGELOG
jonahm-LANL Sep 11, 2024
1f8f708
change comment
jonahm-LANL Sep 13, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [[PR330]](https://github.com/lanl/singularity-eos/pull/330) Piecewise grids for Spiner EOS.

### Fixed (Repair bugs, etc)
- [[PR417]](https://github.com/lanl/singularity-eos/pull/417) Bugs in shared memory related to eospac resolved
- [[PR330]](https://github.com/lanl/singularity-eos/pull/330) Includes a fix for extrapolation of specific internal energy in SpinerEOS.
- [[PR401]](https://github.com/lanl/singularity-eos/pull/401) Fix for internal energy scaling in PTE closure
- [[PR403]](https://github.com/lanl/singularity-eos/pull/403) Fix Gruneisen EOS DensityEnergyFromPressureTemperature function
Expand Down
4 changes: 3 additions & 1 deletion cmake/singularity-eos/hdf5.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ macro(singularity_enable_hdf5 target)

target_include_directories(${target} SYSTEM INTERFACE ${HDF5_INCLUDE_DIRS})
target_link_libraries(${target} INTERFACE ${HDF5_LIBRARIES} ${HDF5_HL_LIBRARIES})

target_link_libraries(${target} INTERFACE dl)
target_link_libraries(${target} INTERFACE z)

if(HDF5_IS_PARALLEL)
# find_package(MPI COMPONENTS C CXX REQUIRED)
# target_link_libraries(${target} INTERFACE MPI::MPI_C MPI::MPI_CXX)
Expand Down
2 changes: 2 additions & 0 deletions singularity-eos/closure/kinetic_phasetransition_methods.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#ifndef _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_METHODS_
#define _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_METHODS_
#ifdef SINGULARITY_BUILD_CLOSURE

#include <ports-of-call/portability.hpp>
#include <ports-of-call/portable_errors.hpp>
Expand Down Expand Up @@ -97,4 +98,5 @@ PORTABLE_INLINE_FUNCTION void SmallStepMFUpdate(const Real logdt, const int num_

} // namespace singularity

#endif // SINGULARITY_BUILD_CLOSURE
#endif // _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_METHODS_
3 changes: 3 additions & 0 deletions singularity-eos/closure/kinetic_phasetransition_models.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#ifndef _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_MODELS_
#define _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_MODELS_

#ifdef SINGULARITY_BUILD_CLOSURE

#include <ports-of-call/portability.hpp>
#include <ports-of-call/portable_errors.hpp>
#include <singularity-eos/base/fast-math/logs.hpp>
Expand Down Expand Up @@ -94,4 +96,5 @@ PORTABLE_INLINE_FUNCTION Real LogMaxTimeStep(const int num_phases, const Real *m

} // namespace singularity

#endif // SINGULARITY_BUILD_CLOSURE
#endif // _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_MODELS_
3 changes: 3 additions & 0 deletions singularity-eos/closure/kinetic_phasetransition_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#ifndef _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_UTILS_
#define _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_UTILS_

#ifdef SINGULARITY_BUILD_CLOSURE

#include <ports-of-call/portability.hpp>
#include <ports-of-call/portable_errors.hpp>
#include <singularity-eos/base/fast-math/logs.hpp>
Expand Down Expand Up @@ -58,4 +60,5 @@ PORTABLE_INLINE_FUNCTION void SortGibbs(const int num_phases, const Real *gibbs,

} // namespace singularity

#endif // SINGULARITY_BUILD_CLOSURE
#endif // _SINGULARITY_EOS_CLOSURE_KINETIC_PHSETRANSITION_UTILS_
7 changes: 3 additions & 4 deletions singularity-eos/eos/eos_eospac.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1251,15 +1251,14 @@ inline std::size_t EOSPAC::SetDynamicMemory(char *src, const SharedMemSettings &
static_assert(sizeof(char) == sizeof(EOS_CHAR), "EOS_CHAR is one byte");
EOS_INTEGER NTABLES[] = {NT};
EOS_INTEGER error_code = EOS_OK;
eosCheckError(error_code, "eos_DestroyAll", Verbosity::Debug);
#ifdef SINGULARITY_EOSPAC_ENABLE_SHARED_MEMORY
PORTABLE_ALWAYS_REQUIRE(
stngs.data != nullptr,
"EOSPAC with shared memory active requires a shared memory pointer");
// JMM: EOS_BOOLEAN is an enum with EOS_FALSE=0 and EOS_TRUE=1.
// EOS_BOOLEAN is not a type alias
EOS_BOOLEAN root = stngs.is_domain_root ? EOS_TRUE : EOS_FALSE;
Yurlungur marked this conversation as resolved.
Show resolved Hide resolved
eos_SetSharedPackedTables(NTABLES, &packed_size_, (EOS_CHAR *)src,
(EOS_CHAR *)stngs.data, stngs.is_node_root, tablehandle,
&error_code);
(EOS_CHAR *)stngs.data, root, tablehandle, &error_code);
#else
eos_SetPackedTables(NTABLES, &packed_size_, (EOS_CHAR *)src, tablehandle, &error_code);
#endif // SINGULARITY_EOSPAC_ENABLE_SHARED_MEMORY
Expand Down
25 changes: 15 additions & 10 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ add_executable(
test_eos_stellar_collapse.cpp
)

add_executable(
closure_unit_tests
catch2_define.cpp
test_closure_pte.cpp
test_kpt_models.cpp
)
if (SINGULARITY_BUILD_CLOSURE)
add_executable(
closure_unit_tests
catch2_define.cpp
test_closure_pte.cpp
test_kpt_models.cpp
)
endif()
Comment on lines +50 to +57
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I added the closure unit tests, I was copying the approach for the tabulated unit tests and trying to simplify the CMake so that each option gets applied to its relevant executables one-by-one. Is it better to complicate the CMake logic to avoid compiling the test rather than rely on #ifdef statements to make sure its basically an empty test?

Put another way, in general should we be trying to push more logic into the CMake and less into the source via preprocessor flags?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh I don't know. Either is fine. I went overboard here, the tests weren't wrapped in preprocessor protection in this case or protected in the cmake so I just did both to be safe.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm pretty ignorant to the finer points of CMake so I'm happy to go either way. But if there's a superior way, let me know. @mauneyc-LANL do you have opinions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍 Yeah happy to revise this structure if there's one that makes sense.


get_property(plugin_tests GLOBAL PROPERTY PLUGIN_TESTS)
if (plugin_tests)
Expand All @@ -71,7 +73,6 @@ endif()

if(SINGULARITY_TEST_SESAME)
target_compile_definitions(eos_tabulated_unit_tests PRIVATE SINGULARITY_TEST_SESAME)
target_compile_definitions(closure_unit_tests PRIVATE SINGULARITY_TEST_SESAME)
endif()
if(SINGULARITY_TEST_STELLAR_COLLAPSE)
target_compile_definitions(eos_tabulated_unit_tests
Expand All @@ -84,8 +85,6 @@ target_link_libraries(eos_infrastructure_tests PRIVATE Catch2::Catch2
singularity-eos::singularity-eos)
target_link_libraries(eos_tabulated_unit_tests PRIVATE Catch2::Catch2
singularity-eos::singularity-eos)
target_link_libraries(closure_unit_tests PRIVATE Catch2::Catch2
singularity-eos::singularity-eos)
if (plugin_tests)
target_link_libraries(eos_plugin_tests PRIVATE Catch2::Catch2
singularity-eos::singularity-eos)
Expand All @@ -94,7 +93,6 @@ include(Catch)
catch_discover_tests(eos_analytic_unit_tests PROPERTIES TIMEOUT 60)
catch_discover_tests(eos_infrastructure_tests PROPERTIES TIMEOUT 60)
catch_discover_tests(eos_tabulated_unit_tests PROPERTIES TIMEOUT 60)
catch_discover_tests(closure_unit_tests PROPERTIES TIMEOUT 120)
if (plugin_tests)
catch_discover_tests(eos_plugin_tests PROPERTIES TIMEOUT 60)
endif()
Expand All @@ -106,6 +104,13 @@ if(SINGULARITY_USE_EOSPAC AND SINGULARITY_TEST_SESAME)
endif()

if(SINGULARITY_BUILD_CLOSURE)
if (SINGULARITY_TEST_SESAME)
target_compile_definitions(closure_unit_tests PRIVATE SINGULARITY_TEST_SESAME)
endif()
target_link_libraries(closure_unit_tests PRIVATE Catch2::Catch2
singularity-eos::singularity-eos)
catch_discover_tests(closure_unit_tests PROPERTIES TIMEOUT 120)

if(SINGULARITY_USE_SPINER)
add_executable(test_pte test_pte.cpp)
target_link_libraries(test_pte PRIVATE Catch2::Catch2
Expand Down
17 changes: 17 additions & 0 deletions test/test_eos_tabulated.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,12 +269,16 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
SpinerEOSDependsRhoT rhoT_orig = SpinerEOSDependsRhoT(eosName, steelID);
SpinerEOSDependsRhoSie rhoSie_orig = SpinerEOSDependsRhoSie(eosName, steelID);
EOS eospac_orig = EOSPAC(steelID);
// we want to stress test that we can serialize and deserialize
// multiple EOSPAC objects
EOS eospac_air = EOSPAC(airID);
THEN("They report dynamic vs static memory correctly") {
REQUIRE(rhoT_orig.AllDynamicMemoryIsShareable());
REQUIRE(rhoSie_orig.AllDynamicMemoryIsShareable());
REQUIRE(!eospac_orig.AllDynamicMemoryIsShareable());
REQUIRE(eospac_orig.SerializedSizeInBytes() >
eospac_orig.DynamicMemorySizeInBytes());
REQUIRE(eospac_air.SerializedSizeInBytes() > eospac_air.DynamicMemorySizeInBytes());
}
WHEN("We serialize") {
auto [rhoT_size, rhoT_data] = rhoT_orig.Serialize();
Expand All @@ -286,6 +290,9 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
auto [eospac_size, eospac_data] = eospac_orig.Serialize();
REQUIRE(eospac_size == eospac_orig.SerializedSizeInBytes());

auto [air_size, air_data] = eospac_air.Serialize();
REQUIRE(air_size == eospac_air.SerializedSizeInBytes());

const std::size_t rhoT_shared_size = rhoT_orig.DynamicMemorySizeInBytes();
REQUIRE(rhoT_size > rhoT_shared_size);

Expand All @@ -295,6 +302,9 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
const std::size_t eospac_shared_size = eospac_orig.DynamicMemorySizeInBytes();
REQUIRE(eospac_size > eospac_shared_size);

const std::size_t air_shared_size = eospac_air.DynamicMemorySizeInBytes();
REQUIRE(air_size > air_shared_size);

THEN("We can deserialize into shared memory") {
using singularity::SharedMemSettings;
using RhoTTricks = singularity::table_utils::SpinerTricks<SpinerEOSDependsRhoT>;
Expand All @@ -304,6 +314,7 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
char *rhoT_shared_data = (char *)malloc(rhoT_shared_size);
char *rhoSie_shared_data = (char *)malloc(rhoSie_shared_size);
char *eospac_shared_data = (char *)malloc(eospac_shared_size);
char *air_shared_data = (char *)malloc(air_shared_size);

SpinerEOSDependsRhoT eos_rhoT;
std::size_t read_size_rhoT =
Expand All @@ -323,6 +334,12 @@ SCENARIO("SpinerEOS and EOSPAC Serialization",
eospac_data, SharedMemSettings(eospac_shared_data, true));
REQUIRE(read_size_eospac == eospac_size);

eospac_air.Finalize();
EOS eos_air_2 = EOSPAC();
std::size_t read_size_air =
eos_air_2.DeSerialize(air_data, SharedMemSettings(air_shared_data, true));
REQUIRE(read_size_air == air_size);

AND_THEN("EOS lookups work") {
constexpr Real rho_trial = 1;
constexpr Real sie_trial = 1e12;
Expand Down
Loading