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

Extract restart read/write IO from M2ulPhyS #243

Merged
merged 14 commits into from
Feb 2, 2024
Merged

Extract restart read/write IO from M2ulPhyS #243

merged 14 commits into from
Feb 2, 2024

Conversation

trevilo
Copy link
Contributor

@trevilo trevilo commented Jan 30, 2024

The goal of this PR is to refactor the restart file IO capabilities currently housed within M2ulPhyS so that they may be better used by other classes in the future. Specifically, we have moved the bulk of the data read/write functions into the IODataOrganizer and IOFamily classes. The parts that remain within M2ulPhyS are metadata read/write capabilities that, at least in principle, could change in other classes and therefore should remain class-specific.

Merging this PR closes #244.

Based on kevin's comments, we don't use or maintain this anymore, so
better get rid of it.
On the path to making the IO capabilities more reusable, in this
commit we extract 5 functions out of M2ulPhyS and into standalone
functions.  The refactored functions are

write_soln_data
read_partitioned_soln_data
read_serialized_soln_data
partitioning_file_hdf5
serialize_soln_for_write
This is an intermediate commit.  The idea is to extract the most of
the read and write functionality from M2ulPhyS::restart_files_hdf5
into separate functions.  In the previous state the read and write
code were intertwined, which made it difficult to understand.  The
eventual goal is to move the parts that are generic into
IODataOrganizer (and related classes), in order to minimize the
restart read/write code within M2ulPhyS, which should make it easier
to reuse these capabilities for other Solver types down the road.
This involves 3 functions that now perform the bulk of the work.
First, the (previously standalone) function serialize_soln_for_write
is moved into the IOFamily class, as serializeForWrite.  Second, two
new functions (IODataOrganizer::write and
IODataOrganizer::writeSerial) are introduced to handle writing field
data that has been registered through the IODataOrganizer class.

At the moment these functions are separate b/c they require different
input data (i.e., writeSerial needs information, such as the
partitioning array, that write does not).  They could be unified,
potentially by extending IODataOrganizer::initializeSerial to obtain
this extra information rather than passing it directly to writeSerial.
But, this refactor is left for later.
This is mostly analogous to the write support from the previous
commit, with new read functions being added to IODataOrganizer.

But, in addition we have to support the option to read data
from a different order solution (to support restarting from lower
order fields).  This is facilitated by adding "aux" objects
(FiniteElementCollection FiniteElementSpace, and ParGridFunction) to
the IOFamily class.  In the previous implementation, these were
instantiated directly within the read function and were specific to
the DG discretization used by M2ulPhyS.  The new implementation should
be generic (although that generality is not yet tested).
@trevilo trevilo self-assigned this Jan 31, 2024
@trevilo trevilo added the enhancement New feature or request label Jan 31, 2024
@trevilo trevilo linked an issue Jan 31, 2024 that may be closed by this pull request
)

The distinctions between write and writeSerial are small, so it is
better to handle both cases through a single function, which we do now
through IODataOrganizer::write.  The IODataOrganizer::writeSerial
function is no longer necessary and is therefore eliminated.
Refactor functions in IODataOrganizer so that capabilities previously
distributed between read, readSerial, and readChangeOrder are all now
handled by a single read function.
The logic in IODataOrganizer::read is hard to follow in big chunks, so
move those big chunks into the IOFamily class, which is where they
belong.  Also, move IOVar vector into IOFamily, which simplifies the
code slightly, and just makes more sense than storing the info in
IODataOrganizer where we have to map from the family name to the
variables.
Now the methods IOFamily::writePartitioned and IOFamily::writeSerial
handle the data write, making the logic in IODataOrganizer::write
easier to follow.
And rename to readDistributeSerializedVariable.  This function needed
the IOFamily info, so makes more sense as a class method.
To reduce duplicated code in size consistency checks.  Also, some
other minor cleaning up.
Plus a couple name changes to be a tiny bit more descriptive
Trailing underscore indicates class member variable.
@trevilo trevilo marked this pull request as ready for review February 1, 2024 20:45
@trevilo trevilo merged commit 48a1d20 into main Feb 2, 2024
15 of 16 checks passed
@trevilo trevilo deleted the io-extract branch March 26, 2024 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor IO in M2ulPhyS
1 participant