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.

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

Test serialization for serial and shared triangulations #15649

Merged
merged 5 commits into from Aug 1, 2023

Conversation

pcafrica
Copy link
Contributor

@pcafrica pcafrica commented Jul 4, 2023

Depends on #15576.

Tests for serializing/deserializing were already implemented for p::d::T and p::f::T.

The new design introduced in #15576 unlocks the same interface also for T and p::s::T.

This PR adds 3 new tests for serializing/deserializing:

  • a serial triangulation
  • a serial triangulation and an associated FE vector
  • a shared triangulation and an associated FE vector, on 2 MPI ranks

@peterrum @blaisb @bangerth

@tamiko
Copy link
Member

tamiko commented Jul 6, 2023

/rebuild

@pcafrica pcafrica force-pushed the serialization_test branch 2 times, most recently from 995e99e to dbea6d7 Compare July 9, 2023 18:43
@peterrum
Copy link
Member

peterrum commented Jul 9, 2023

@pcafrica After you rebase, I'll take a look at this PR.

@pcafrica
Copy link
Contributor Author

pcafrica commented Jul 9, 2023

Thank you @peterrum! I have just rebased, this MR is ready for review.

#include "./tests.h"
#include "../grid/tests.h"
Copy link
Member

@peterrum peterrum Jul 9, 2023

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

@pcafrica pcafrica Jul 10, 2023

Choose a reason for hiding this comment

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

The tests.h file has just been moved from fullydistributed_grids to grids, without modify its content.

Is it fine to reference it also from other folders than grids?

tests/grid/tests.h Outdated Show resolved Hide resolved
{
boost::archive::text_oarchive oa(stream);
oa << triangulation;
oa << vector;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to test here the vector. There should be other tests whether the serialization/deserialization of vectors work.

{
const std::string filename = "save_load_" + std::to_string(dim) + "d_out";

triangulation.save(filename);
Copy link
Member

Choose a reason for hiding this comment

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

But here we should test vectors sometime in the future ;) What is missing for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing is missing, indeed :)

This is actually tested in grids/serialization_01.cc.

std::stringstream stream;
{
boost::archive::text_oarchive oa(stream);
oa << triangulation;
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering what happens for p:s:T for load/save? I guess it should work but users need to provide unique file names on each process!?

Copy link
Contributor Author

@pcafrica pcafrica Jul 10, 2023

Choose a reason for hiding this comment

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

Given the current implementation of T::{save,load}(), the following two are equivalent:

boost::archive::text_oarchive oa(std::ofstream(filename));
oa << triangulation;

and

triangulation.save(filename);

I decided to stick with the first one, as it is more versatile (we can easily serialize more information, such as vectors, onto the same file). Also, @blaisb and I are planning to use it for step-83.

The p::s::T::{save,load}() methods were already tested (see sharedtria/tria_load_01.cc), and #15576 did not break it.

So if run in parallel then yes, unique file names must be provided on each process.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding step-83: I think triangulation.save(filename); is the right approach.

@peterrum
Copy link
Member

peterrum commented Jul 9, 2023

@pcafrica One thing I am wondering of is whether parallel:distributed::SolutionTransfer now also works serial triangulation (at least the serialization/deserialization part).

@pcafrica
Copy link
Contributor Author

@pcafrica One thing I am wondering of is whether parallel:distributed::SolutionTransfer now also works serial triangulation (at least the serialization/deserialization part).

Personally, I haven't tested it.

From what I see here, there are a couple of dynamic_cast<p::d::T>s that should be tentatively removed.

I'll try out, and possibly open a new PR.

@peterrum
Copy link
Member

/jenkins/workspace/dealii-serial_PR-15649/tests/serialization/parallel_fullydistributed_construction_data_1.cc:29:10: fatal error: ../fullydistributed_grids/tests.h: No such file or directory
   29 | #include "../fullydistributed_grids/tests.h"

@pcafrica
Copy link
Contributor Author

/jenkins/workspace/dealii-serial_PR-15649/tests/serialization/parallel_fullydistributed_construction_data_1.cc:29:10: fatal error: ../fullydistributed_grids/tests.h: No such file or directory
   29 | #include "../fullydistributed_grids/tests.h"

Fixed, thank you!

@tamiko
Copy link
Member

tamiko commented Jul 24, 2023

@peterrum Do you approve now?

std::stringstream stream;
{
boost::archive::text_oarchive oa(stream);
oa << triangulation;
Copy link
Member

Choose a reason for hiding this comment

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

Regarding step-83: I think triangulation.save(filename); is the right approach.

@peterrum peterrum merged commit dd4023d into dealii:master Aug 1, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants