diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 0ea5fa525f5..8725b431d18 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -29,9 +29,19 @@ 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 - pip install sphinx-multiversion + python -m pip install --upgrade pip + + # 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 + sphinx-multiversion --version || true - name: build code run: | mkdir -p build 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) diff --git a/doc/sphinx/src/using-eos.rst b/doc/sphinx/src/using-eos.rst index ed42b204eb1..fe34a72c500 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 let 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();