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

Refactoring of buffer handling system #324

Merged
merged 21 commits into from
Nov 27, 2024

Conversation

JoMee
Copy link
Contributor

@JoMee JoMee commented Nov 4, 2024

Enhanced Buffer Management with Logging

Summary

This pull request introduces a BufferHandler class along with a LoggingBufferHandler decorator to simplify the buffer management and add logging functionality. As well as added code to the Communicator to gather all logs and store them in a file.

Changes

1. BufferHandler Implementation

  • Created a BufferHandler class for buffer management, without the need for the tag attribute.
  • Added methods for explicitly managing used and free buffers.
  • Doxygen documentation.

2. LoggingBufferHandler Decorator

  • Added a LoggingBufferHandler decorator, which wraps BufferHandler and logs buffer operations.
  • Doxygen documentation.

3. Integration with Communicator Class

  • Integrated the LoggingBufferHandler into the Communicator class.

4. Serialization and Deserialization

  • Implemented serialization and deserialization functions for LogEntry data to support easy handling of log data.
  • Added functionality to gather logs from all MPI ranks and save them in a structured file format.

6. Unit Tests

  • Developed unit tests for BufferHandler and LoggingBufferHandler and serialization, covering core functionalities, logging operations, and serialization/deserialization.
  • Unit tests run for buffer management run for all available memory spaces.

Testing

  • Ran templated tests against multiple memory spaces.
  • Tests have been executed successfully on Gwendolyn on an A100 and on my host system.

…. The removal of the id has caused some unused parameter warnings. I also changed the structure of the BufferHandler a little bit to allow for a decorator pattern for the logger
…xed some bugs in the pointtopoint mpi wrappers for blocking communication
@JoMee JoMee marked this pull request as draft November 5, 2024 15:36
@JoMee JoMee marked this pull request as ready for review November 5, 2024 15:36
@matt-frey matt-frey added the enhancement New feature or request label Nov 5, 2024
src/Particle/ParticleSpatialLayout.hpp Show resolved Hide resolved
src/Communicate/Buffers.hpp Outdated Show resolved Hide resolved
src/Communicate/LoggingBufferHandler.h Outdated Show resolved Hide resolved
test/field/TestHalo.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,117 @@
#include "Ippl.h"
Copy link
Member

Choose a reason for hiding this comment

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

in genera i.e. for all test a high level description of what is tested would be nice for ALL tests

Copy link
Member

@matt-frey matt-frey left a comment

Choose a reason for hiding this comment

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

Looks good in general. Thanks. I have a few comments though.

src/Particle/ParticleSpatialLayout.hpp Show resolved Hide resolved
src/Communicate/BufferHandler.h Outdated Show resolved Hide resolved
src/Communicate/BufferHandler.h Outdated Show resolved Hide resolved
src/Communicate/BufferHandler.h Outdated Show resolved Hide resolved
src/Communicate/BufferHandler.h Outdated Show resolved Hide resolved
src/Communicate/BufferHandler.hpp Outdated Show resolved Hide resolved
src/Communicate/BufferHandler.hpp Outdated Show resolved Hide resolved
src/Communicate/CommunicatorLogging.cpp Outdated Show resolved Hide resolved
src/Communicate/LoggingBufferHandler.hpp Outdated Show resolved Hide resolved
test/field/TestHalo.cpp Outdated Show resolved Hide resolved
@matt-frey matt-frey added bug Something isn't working and removed bug Something isn't working labels Nov 6, 2024
Copy link
Member

@matt-frey matt-frey left a comment

Choose a reason for hiding this comment

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

Thanks!

@JoMee JoMee requested a review from aaadelmann November 7, 2024 11:42
@JoMee JoMee merged commit 21149e2 into IPPL-framework:master Nov 27, 2024
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.

3 participants