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

Trilinos Import utility add restart #2285

Merged
merged 10 commits into from
Jun 18, 2018
Merged

Conversation

philbucher
Copy link
Member

This PR adds the restart loading option to the TrilinosImportModelPartUtility

I also updated the solvers in Fluid and Structure

else:
raise Exception("Other input options are not yet implemented.")

def CreateCommunicators(self):
## Construct and execute the MPICommunicator
KratosMetis.SetMPICommunicatorProcess(self.main_model_part).Execute()
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that this can be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The metis process only (and only) replaces the Communicator for the MPICommunicator
=> see here

this is also the first thing the ParallelFillCommunicator does, see here
The ParallelFillCommunicator also computes the graphs for the SubModelParts according to @jcotela

Copy link
Member

Choose a reason for hiding this comment

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

Perfect! Thanks for the explanation @philbucher

@@ -108,7 +108,7 @@ def ImportModelPart(self):
## Construct the Trilinos import model part utility
self.trilinos_model_part_importer = trilinos_import_model_part_utility.TrilinosImportModelPartUtility(self.main_model_part, self.settings)
## Execute the Metis partitioning and reading
self.trilinos_model_part_importer.ExecutePartitioningAndReading()
self.trilinos_model_part_importer.ImportModelPart()
Copy link
Member

Choose a reason for hiding this comment

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

IMO ExecutePartitioningAndReading is a much more self-explicative name of what this method does.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it bcs now the utility can also do restarts (and possibly also other imports, think of hdf5,...)
This way whenever a new functionality is being added in this utility it will be also available for the solvers.

Then the name ExecutePartitioningAndReading does not fit any more

Copy link
Member

Choose a reason for hiding this comment

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

Go ahead then!

Copy link
Member

Choose a reason for hiding this comment

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

@philbucher are we sure we want to add all the new functionalities to the same utility? I'd rather have separate classes to read mdpa, hdf5 and restart files, for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jcotela this is what I would like to avoid, otherwise we will have a lot of copy-pasting with more options becoming available

Copy link
Member

Choose a reason for hiding this comment

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

@philbucher I agree in principle, but my preferred solution to that would be a good I/O base class all these utilities can derive from... I guess we will see what works when we get there though...

Copy link
Member Author

Choose a reason for hiding this comment

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

I talked to @jcotela and we agreed that we leave the proposed solution and change it in the future to e.g. a factory

@philbucher
Copy link
Member Author

ping

@philbucher
Copy link
Member Author

I pushed some fixes, now everything should work smoothly.

@philbucher
Copy link
Member Author

can we merge this?

@philbucher
Copy link
Member Author

thanks

@philbucher philbucher merged commit 59a3239 into master Jun 18, 2018
@philbucher philbucher deleted the core/mpi-import-modelpart branch June 18, 2018 14:32
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.

3 participants