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

Moved device initialization to the very beginning #879

Merged
merged 2 commits into from Jun 8, 2023

Conversation

ravil-mobile
Copy link
Collaborator

Also

  • Updated reference to device
  • applied formatting to SeisSol.cpp, SeisSol.h and main.cpp
  • edited info printing in ElementWiseBuilder.hpp

Also
* Updated reference to device
* applied formatting to `SeisSol.cpp`, `SeisSol.h` and `main.cpp`
* edited info printing in `ElementWiseBuilder.hpp`
Copy link
Contributor

@davschneller davschneller left a comment

Choose a reason for hiding this comment

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

Just some tiny comments. Looking good otherwise.

@@ -35,8 +35,9 @@ class ElementWiseBuilder : public ReceiverBasedOutputBuilder {
const auto numFaultElements = meshReader->getFault().size();
const auto numSubTriangles = faultRefiner->getNumSubTriangles();

logInfo(localRank) << "CPP: Initialising Fault output. "
<< "Number of sub-triangles: " << numSubTriangles;
logInfo(localRank) << "Initialising Fault output."
Copy link
Contributor

Choose a reason for hiding this comment

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

This may be a more general question... British or American spelling?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to Initializing

case utils::Args::Success:
break;
break;
}

// Initialize the ASYNC I/O library
if (!m_asyncIO.init())
Copy link
Contributor

Choose a reason for hiding this comment

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

{ return false; }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

*/
void setMeshReader(seissol::geometry::MeshReader* meshReader) {
if (m_meshReader != 0L)
logError() << "Mesh reader already initialized";
Copy link
Contributor

Choose a reason for hiding this comment

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

{logError() << "Mesh reader already initialized";}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/SeisSol.h Outdated
* Set the mesh reader
*/
void setMeshReader(seissol::geometry::MeshReader* meshReader) {
if (m_meshReader != 0L)
Copy link
Contributor

Choose a reason for hiding this comment

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

!= nullptr

(maybe longer-term—not necessarily this PR—make a managed pointer?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

int main(int argc, char* argv[])
{
LIKWID_MARKER_INIT;
#ifdef ACL_DEVICE
Copy link
Contributor

Choose a reason for hiding this comment

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

Merge with the #if from above? I.e.

#ifdef ACL_DEVICE
#ifdef USE_MPI
MPI::mpi.bindAcceleratorDevice();
#endif
  device::DeviceInstance& device = device::DeviceInstance::getInstance();
  device.api->initialize();
  device.api->allocateStackMem();
#endif

or so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@ravil-mobile
Copy link
Collaborator Author

@davschneller thanks for the review.

In SeisSol.h, I moved all public methods to the top and private once to the bottom

@davschneller davschneller added this pull request to the merge queue Jun 8, 2023
Merged via the queue into master with commit c2062e4 Jun 8, 2023
2 of 5 checks passed
@davschneller davschneller deleted the ravil/device-init branch August 4, 2023 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants