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

#94: Remove use of MPI_COMM_WORLD in Krino package #108

Closed

Conversation

thearusable
Copy link
Collaborator

Close #94

@@ -84,7 +84,7 @@ Startup::Startup(int argc, char ** argv)
throw std::runtime_error("MPI_Init failed");
}

stk::EnvData::instance().m_parallelComm = MPI_COMM_WORLD;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function calls MPI_Init so it makes sense to use MPI_COMM_WORLD here (there's no comm split happening)

@@ -75,7 +76,7 @@ class BoundingBoxMesh {
public:
BoundingBoxMesh(stk::topology element_topology, const std::vector<std::string>& rank_names = std::vector<std::string>());
void set_domain(const BoundingBoxType & mesh_bbox, const double mesh_size, const int pad_cells = 0);
void populate_mesh(stk::ParallelMachine pm = MPI_COMM_WORLD, const stk::mesh::BulkData::AutomaticAuraOption auto_aura_option = stk::mesh::BulkData::AUTO_AURA);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think having MPI_COMM_WORLD as default value is ok, if the user wants, he'll just use different comm

@JacobDomagala
Copy link
Collaborator

I'm ok with the changes, but I think the use of MPI_COMM_WORLD is justified here.

@thearusable
Copy link
Collaborator Author

@JacobDomagala Ok, in this case I think it would be good to close this PR.

@stmcgovern What do you think?

@cwschilly cwschilly force-pushed the 94-krino-remove-use-of-MPI_COMM_WORLD branch from ed68bf9 to 3c3acfe Compare August 18, 2023 20:10
@stmcgovern
Copy link
Collaborator

Default argument use of MPI_COMM_WORLD in Krino is fine.

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.

Krino: Remove the use of MPI_COMM_WORLD
3 participants