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

Refactor IO in M2ulPhyS #244

Closed
trevilo opened this issue Jan 31, 2024 · 0 comments · Fixed by #243
Closed

Refactor IO in M2ulPhyS #244

trevilo opened this issue Jan 31, 2024 · 0 comments · Fixed by #243
Assignees
Labels
enhancement New feature or request

Comments

@trevilo
Copy link
Contributor

trevilo commented Jan 31, 2024

A large portion of IO code currently lives in the M2ulPhyS class, making it impossible to reuse for other solvers. We need refactor this code to 1) extract the parts that are reusable into a more generic capability and 2) isolate the parts that are M2ulPhyS specific.

@trevilo trevilo added the enhancement New feature or request label Jan 31, 2024
@trevilo trevilo self-assigned this Jan 31, 2024
trevilo added a commit that referenced this issue Jan 31, 2024
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
trevilo added a commit that referenced this issue Jan 31, 2024
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.
trevilo added a commit that referenced this issue Jan 31, 2024
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.
trevilo added a commit that referenced this issue Jan 31, 2024
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 linked a pull request Jan 31, 2024 that will close this issue
trevilo added a commit that referenced this issue Jan 31, 2024
)

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.
trevilo added a commit that referenced this issue Feb 1, 2024
Refactor functions in IODataOrganizer so that capabilities previously
distributed between read, readSerial, and readChangeOrder are all now
handled by a single read function.
trevilo added a commit that referenced this issue Feb 1, 2024
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.
trevilo added a commit that referenced this issue Feb 1, 2024
Now the methods IOFamily::writePartitioned and IOFamily::writeSerial
handle the data write, making the logic in IODataOrganizer::write
easier to follow.
trevilo added a commit that referenced this issue Feb 1, 2024
And rename to readDistributeSerializedVariable.  This function needed
the IOFamily info, so makes more sense as a class method.
trevilo added a commit that referenced this issue Feb 1, 2024
To reduce duplicated code in size consistency checks.  Also, some
other minor cleaning up.
trevilo added a commit that referenced this issue Feb 1, 2024
To reduce duplicated code in size consistency checks.  Also, some
other minor cleaning up.
trevilo added a commit that referenced this issue Feb 1, 2024
Plus a couple name changes to be a tiny bit more descriptive
trevilo added a commit that referenced this issue Feb 2, 2024
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
trevilo added a commit that referenced this issue Feb 2, 2024
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.
trevilo added a commit that referenced this issue Feb 2, 2024
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.
trevilo added a commit that referenced this issue Feb 2, 2024
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 added a commit that referenced this issue Feb 2, 2024
)

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.
trevilo added a commit that referenced this issue Feb 2, 2024
Refactor functions in IODataOrganizer so that capabilities previously
distributed between read, readSerial, and readChangeOrder are all now
handled by a single read function.
trevilo added a commit that referenced this issue Feb 2, 2024
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.
trevilo added a commit that referenced this issue Feb 2, 2024
Now the methods IOFamily::writePartitioned and IOFamily::writeSerial
handle the data write, making the logic in IODataOrganizer::write
easier to follow.
trevilo added a commit that referenced this issue Feb 2, 2024
And rename to readDistributeSerializedVariable.  This function needed
the IOFamily info, so makes more sense as a class method.
trevilo added a commit that referenced this issue Feb 2, 2024
To reduce duplicated code in size consistency checks.  Also, some
other minor cleaning up.
trevilo added a commit that referenced this issue Feb 2, 2024
Plus a couple name changes to be a tiny bit more descriptive
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 a pull request may close this issue.

1 participant