From a4fdd2917dc3d5b3f51e35eb29b505946e81b3a0 Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Fri, 16 Jan 2026 11:33:15 -0500 Subject: [PATCH 1/7] change pointer returned by Serialize() to be smart --- doc/sphinx/src/using-eos.rst | 17 ++++++++++++++--- singularity-eos/base/serialization_utils.hpp | 5 +++-- singularity-eos/eos/eos_base.hpp | 5 +++-- singularity-eos/eos/eos_variant.hpp | 5 +++-- test/test_eos_helmholtz.cpp | 4 +++- test/test_eos_ideal.cpp | 11 +++++------ test/test_eos_modifiers_minimal.cpp | 4 +--- test/test_eos_stellar_collapse.cpp | 16 +++++++--------- test/test_eos_tabulated.cpp | 15 ++++++--------- 9 files changed, 45 insertions(+), 37 deletions(-) diff --git a/doc/sphinx/src/using-eos.rst b/doc/sphinx/src/using-eos.rst index ed42b204eb1..6a1e04bfa4f 100644 --- a/doc/sphinx/src/using-eos.rst +++ b/doc/sphinx/src/using-eos.rst @@ -81,11 +81,20 @@ The function fills the ``dst`` pointer with the memory required for serialization and returns the number of bytes written to ``dst``. The function -.. cpp:function:: std::pair EOS::Serialize(); +.. cpp:function:: std::pair> EOS::Serialize(); -allocates a ``char*`` pointer to contain serialized data and fills +allocates a ``std::shared_ptr`` to contain serialized data and fills it. +.. note:: + + Note that ``Serialize()`` uses a smart pointer while most other + ``singularity-eos`` machinery works with unmanaged pointers. This + means the pointer returned by ``Serialize()`` does not need to be + freed. However it is the responsiblity of the host code to manage + the memory for other pointers that the serialization machinery may + utilize or interact with. + .. warning:: Serialization and de-serialization may only be performed on objects @@ -141,7 +150,9 @@ For example you might call ``DeSerialize`` as would call ``eos.Finalize()``. If the ``SharedMemSettings`` are utilized to request data be written to a shared memory pointer, however, you can free the ``src`` pointer, so long as you don't free - the shared memory pointer. + the shared memory pointer. This means you must manage these pointers + externally and not them go out of scope, especially if you use smart + pointers. Putting everything together, a full sequence with MPI might look like this: diff --git a/singularity-eos/base/serialization_utils.hpp b/singularity-eos/base/serialization_utils.hpp index 2c3028d54f5..df50c8fe4a2 100644 --- a/singularity-eos/base/serialization_utils.hpp +++ b/singularity-eos/base/serialization_utils.hpp @@ -15,6 +15,7 @@ #ifndef SINGULARITY_EOS_BASE_SERIALIZATION_UTILS_ #define SINGULARITY_EOS_BASE_SERIALIZATION_UTILS_ +#include #include #include #include @@ -67,8 +68,8 @@ struct BulkSerializer { } auto Serialize() { std::size_t size = SerializedSizeInBytes(); - char *dst = (char *)malloc(size); - std::size_t new_size = Serialize(dst); + std::shared_ptr dst(new char[size]); + std::size_t new_size = Serialize(dst.get()); PORTABLE_ALWAYS_REQUIRE(size == new_size, "Serialization failed!"); return std::make_pair(size, dst); } diff --git a/singularity-eos/eos/eos_base.hpp b/singularity-eos/eos/eos_base.hpp index 2e8581454b4..11cc5d261eb 100644 --- a/singularity-eos/eos/eos_base.hpp +++ b/singularity-eos/eos/eos_base.hpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -1052,8 +1053,8 @@ class EosBase { auto Serialize() { CRTP *pcrtp = static_cast(this); std::size_t size = pcrtp->SerializedSizeInBytes(); - char *dst = (char *)malloc(size); - std::size_t size_new = Serialize(dst); + std::shared_ptr dst(new char[size]); + std::size_t size_new = Serialize(dst.get()); PORTABLE_ALWAYS_REQUIRE(size_new == size, "Serialization failed!"); return std::make_pair(size, dst); } diff --git a/singularity-eos/eos/eos_variant.hpp b/singularity-eos/eos/eos_variant.hpp index d04b7c3674f..691b6ad91ca 100644 --- a/singularity-eos/eos/eos_variant.hpp +++ b/singularity-eos/eos/eos_variant.hpp @@ -17,6 +17,7 @@ #ifndef EOS_VARIANT_HPP #define EOS_VARIANT_HPP +#include #include #include #include @@ -1336,8 +1337,8 @@ class Variant { } auto Serialize() { std::size_t size = SerializedSizeInBytes(); - char *dst = (char *)malloc(size); - std::size_t new_size = Serialize(dst); + std::shared_ptr dst(new char[size]); + std::size_t new_size = Serialize(dst.get()); PORTABLE_ALWAYS_REQUIRE(size == new_size, "Serialization failed!"); return std::make_pair(size, dst); } diff --git a/test/test_eos_helmholtz.cpp b/test/test_eos_helmholtz.cpp index 037835cacaf..d3735c2ea24 100644 --- a/test/test_eos_helmholtz.cpp +++ b/test/test_eos_helmholtz.cpp @@ -47,7 +47,7 @@ SCENARIO("Helmholtz equation of state - Table interpolation (tgiven)", "[Helmhol Helmholtz host_eos_2; auto [size, data] = host_eos.Serialize(); - auto read_size = host_eos_2.DeSerialize(data); + auto read_size = host_eos_2.DeSerialize(data.get()); THEN("We can serialize!") { host_eos_2.CheckParams(); REQUIRE(size == read_size); @@ -158,6 +158,8 @@ SCENARIO("Helmholtz equation of state - Table interpolation (tgiven)", "[Helmhol REQUIRE(nwrong == 0); eos.Finalize(); host_eos.Finalize(); + eos_2.Finalize(); + host_eos_2.Finalize(); } } diff --git a/test/test_eos_ideal.cpp b/test/test_eos_ideal.cpp index 4b90f524894..dd2f5ab8d0d 100644 --- a/test/test_eos_ideal.cpp +++ b/test/test_eos_ideal.cpp @@ -300,10 +300,10 @@ SCENARIO("Ideal gas serialization", "[IdealGas][Serialization]") { THEN("We can de-serialize new objects from them") { IdealGas new_bare; - new_bare.DeSerialize(data_bare); + new_bare.DeSerialize(data_bare.get()); EOS new_variant; - new_variant.DeSerialize(data_var); + new_variant.DeSerialize(data_var.get()); AND_THEN("The bare eos has the right Cv and Gruneisen params") { REQUIRE(new_bare.SpecificHeatFromDensityTemperature(1.0, 1.0) == Cv); @@ -318,11 +318,10 @@ SCENARIO("Ideal gas serialization", "[IdealGas][Serialization]") { REQUIRE(new_variant.SpecificHeatFromDensityTemperature(1.0, 1.0) == Cv); REQUIRE(new_variant.GruneisenParamFromDensityTemperature(1.0, 1.0) == gm1); } - } - // cleanup - free(data_bare); - free(data_var); + new_bare.Finalize(); + new_variant.Finalize(); + } } // cleanup diff --git a/test/test_eos_modifiers_minimal.cpp b/test/test_eos_modifiers_minimal.cpp index 41776946f5b..ace79a7406b 100644 --- a/test/test_eos_modifiers_minimal.cpp +++ b/test/test_eos_modifiers_minimal.cpp @@ -206,7 +206,7 @@ SCENARIO("Serialization of modified EOSs preserves their properties", THEN("We can de-serialize the EOS") { singularity::VectorSerializer deserializer; - deserializer.DeSerialize(data); + deserializer.DeSerialize(data.get()); REQUIRE(deserializer.Size() == serializer.Size()); AND_THEN("The de-serialized EOS still evaluates properly") { @@ -216,8 +216,6 @@ SCENARIO("Serialization of modified EOSs preserves their properties", temp_test, EPS)); } } - - free(data); } eos.Finalize(); diff --git a/test/test_eos_stellar_collapse.cpp b/test/test_eos_stellar_collapse.cpp index 7087e4398cb..a0fc54123e6 100644 --- a/test/test_eos_stellar_collapse.cpp +++ b/test/test_eos_stellar_collapse.cpp @@ -308,7 +308,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { REQUIRE(size > 0); THEN("We can de-serialize it into a new object") { StellarCollapse sc2; - std::size_t read_size = sc2.DeSerialize(data); + std::size_t read_size = sc2.DeSerialize(data.get()); REQUIRE(read_size == size); REQUIRE(size > sizeof(StellarCollapse)); sc2.CheckParams(); @@ -320,7 +320,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { } AND_THEN("We can de-serialize into two objects") { StellarCollapse sc3; - std::size_t read_size_2 = sc3.DeSerialize(data); + std::size_t read_size_2 = sc3.DeSerialize(data.get()); REQUIRE(read_size_2 == size); sc3.CheckParams(); AND_THEN("The two de-serialized objects use the same memory") { @@ -335,7 +335,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { char *shared_data = (char *)malloc(shared_size); SharedMemSettings settings(shared_data, true); StellarCollapse sc3; - std::size_t read_size = sc3.DeSerialize(data, settings); + std::size_t read_size = sc3.DeSerialize(data.get(), settings); REQUIRE(read_size == size); REQUIRE(read_size > sizeof(StellarCollapse)); sc3.CheckParams(); @@ -353,7 +353,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { AND_THEN("We can de-serialize from shared memory around one more object") { SharedMemSettings settings2(shared_data, false); StellarCollapse sc4; - std::size_t read_size_2 = sc4.DeSerialize(data, settings); + std::size_t read_size_2 = sc4.DeSerialize(data.get(), settings); REQUIRE(read_size_2 == size); REQUIRE(read_size_2 > sizeof(StellarCollapse)); sc4.CheckParams(); @@ -367,7 +367,6 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { free(shared_data); } } - free(data); } WHEN("We serialize a variant that owns a StellarCollapse EOS") { @@ -378,7 +377,7 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { REQUIRE(size > 0); THEN("We can de-serialize it into a new object") { EOS e2; - std::size_t read_size = e2.DeSerialize(data); + std::size_t read_size = e2.DeSerialize(data.get()); REQUIRE(read_size == size); e2.CheckParams(); AND_THEN("The two stellar collapse EOS's agree") { @@ -394,12 +393,12 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { EOS e3, e4; std::size_t read_size_e3 = - e3.DeSerialize(data, SharedMemSettings(shared_data, true)); + e3.DeSerialize(data.get(), SharedMemSettings(shared_data, true)); REQUIRE(read_size_e3 == size); e3.CheckParams(); std::size_t read_size_e4 = - e4.DeSerialize(data, SharedMemSettings(shared_data, false)); + e4.DeSerialize(data.get(), SharedMemSettings(shared_data, false)); REQUIRE(read_size_e4 == size); e4.CheckParams(); @@ -417,7 +416,6 @@ SCENARIO("Stellar Collapse EOS", "[StellarCollapse]") { free(shared_data); } } - free(data); } sc.Finalize(); } diff --git a/test/test_eos_tabulated.cpp b/test/test_eos_tabulated.cpp index 125945f17ab..e65ffdcbf89 100644 --- a/test/test_eos_tabulated.cpp +++ b/test/test_eos_tabulated.cpp @@ -375,27 +375,27 @@ SCENARIO("SpinerEOS and EOSPAC Serialization", char *air_shared_data = (char *)malloc(air_shared_size); SpinerEOSDependsRhoT eos_rhoT; - std::size_t read_size_rhoT = - eos_rhoT.DeSerialize(rhoT_data, SharedMemSettings(rhoT_shared_data, true)); + std::size_t read_size_rhoT = eos_rhoT.DeSerialize( + rhoT_data.get(), SharedMemSettings(rhoT_shared_data, true)); REQUIRE(read_size_rhoT == rhoT_size); REQUIRE(RhoTTricks::DataBoxesPointToDifferentMemory(rhoT_orig, eos_rhoT)); SpinerEOSDependsRhoSie eos_rhoSie; std::size_t read_size_rhoSie = eos_rhoSie.DeSerialize( - rhoSie_data, SharedMemSettings(rhoSie_shared_data, true)); + rhoSie_data.get(), SharedMemSettings(rhoSie_shared_data, true)); REQUIRE(read_size_rhoSie == rhoSie_size); REQUIRE(RhoSieTricks::DataBoxesPointToDifferentMemory(rhoSie_orig, eos_rhoSie)); eospac_orig.Finalize(); EOS eos_eospac = EOSPAC(); std::size_t read_size_eospac = eos_eospac.DeSerialize( - eospac_data, SharedMemSettings(eospac_shared_data, true)); + eospac_data.get(), 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)); + std::size_t read_size_air = eos_air_2.DeSerialize( + air_data.get(), SharedMemSettings(air_shared_data, true)); REQUIRE(read_size_air == air_size); AND_THEN("EOS lookups work") { @@ -430,9 +430,6 @@ SCENARIO("SpinerEOS and EOSPAC Serialization", free(rhoSie_shared_data); free(eospac_shared_data); } - free(rhoT_data); - free(rhoSie_data); - free(eospac_data); } rhoT_orig.Finalize(); From 8b0249e7f75ac1072fc6634714b0a07e08e2643f Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Fri, 16 Jan 2026 11:37:12 -0500 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 477c17796b4..477720489a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixed (Repair bugs, etc) ### Changed (changing behavior/API/variables/...) +- [[PR601]](https://github.com/lanl/singularity-eos/pull/601) Make Serialize() return a smart pointer ### Infrastructure (changes irrelevant to downstream codes) From a9b149a3a39bf82d2f1aab7f0057f7afae736aaf Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Fri, 16 Jan 2026 11:53:48 -0500 Subject: [PATCH 3/7] typo --- doc/sphinx/src/using-eos.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/sphinx/src/using-eos.rst b/doc/sphinx/src/using-eos.rst index 6a1e04bfa4f..fe34a72c500 100644 --- a/doc/sphinx/src/using-eos.rst +++ b/doc/sphinx/src/using-eos.rst @@ -151,8 +151,8 @@ For example you might call ``DeSerialize`` as utilized to request data be written to a shared memory pointer, however, you can free the ``src`` pointer, so long as you don't free the shared memory pointer. This means you must manage these pointers - externally and not them go out of scope, especially if you use smart - pointers. + externally and not let them go out of scope, especially if you use + smart pointers. Putting everything together, a full sequence with MPI might look like this: From 4786d8a1331583f9d0d465b1ce5a545ea06204c8 Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Tue, 20 Jan 2026 10:37:31 -0500 Subject: [PATCH 4/7] try installing newer sphinx multiversion from git --- .github/workflows/docs.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 0ea5fa525f5..79bf2213970 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -31,7 +31,8 @@ jobs: sudo apt-get install -y --force-yes -qq build-essential python3-dev pip install sphinx pip install sphinx-rtd-theme - pip install sphinx-multiversion + #pip install sphinx-multiversion + pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" - name: build code run: | mkdir -p build From bbdb1aa9ce933f18ec7fd5cae7c4d88464a1fd53 Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Tue, 20 Jan 2026 10:48:21 -0500 Subject: [PATCH 5/7] try using sphinx-multiversion-contrib --- .github/workflows/docs.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 79bf2213970..32847af6fe3 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -29,10 +29,12 @@ jobs: run: | sudo apt-get update -y -qq sudo apt-get install -y --force-yes -qq build-essential python3-dev - pip install sphinx - pip install sphinx-rtd-theme + python -m pip install --upgrade pip + python -m pip install sphinx + python -m pip install sphinx-rtd-theme + python -m pip install sphinx-multiversion-contrib #pip install sphinx-multiversion - pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" + #pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" - name: build code run: | mkdir -p build From ca0a66689947e1ef746bb12833aec62023ebab04 Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Tue, 20 Jan 2026 10:49:18 -0500 Subject: [PATCH 6/7] Output version --- .github/workflows/docs.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 32847af6fe3..e09dac790ad 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -35,6 +35,10 @@ jobs: python -m pip install sphinx-multiversion-contrib #pip install sphinx-multiversion #pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" + + python -m pip show sphinx sphinx-multiversion + sphinx-build --version + sphinx-multiversion --version || true - name: build code run: | mkdir -p build From ddc648a9316ec4676f680a3c837fd1d70280a1bb Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Tue, 20 Jan 2026 10:57:05 -0500 Subject: [PATCH 7/7] lets try this --- .github/workflows/docs.yml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index e09dac790ad..8725b431d18 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -30,11 +30,14 @@ jobs: sudo apt-get update -y -qq sudo apt-get install -y --force-yes -qq build-essential python3-dev python -m pip install --upgrade pip - python -m pip install sphinx - python -m pip install sphinx-rtd-theme - python -m pip install sphinx-multiversion-contrib - #pip install sphinx-multiversion - #pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" + + # Pin sphinx to an older version until sphinx-multiversion is fixed + # see + # https://github.com/sphinx-contrib/multiversion/pull/202 + python -m pip install "sphinx<8.2" sphinx-rtd-theme + + # This should pull in the fixed version of multiversion when its available + python -m pip install "sphinx-multiversion @ git+https://github.com/sphinx-contrib/multiversion.git" python -m pip show sphinx sphinx-multiversion sphinx-build --version