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

Caliper bindings are broken #4740

Closed
jngrad opened this issue Jun 1, 2023 · 0 comments · Fixed by #4747
Closed

Caliper bindings are broken #4740

jngrad opened this issue Jun 1, 2023 · 0 comments · Fixed by #4747
Assignees
Labels

Comments

@jngrad
Copy link
Member

jngrad commented Jun 1, 2023

The Caliper bindings that mark the start and end of a portion of code to instrument are incorrect:

inline void begin_section(const std::string &name) {
ESPRESSO_PROFILER_MARK_BEGIN(name.c_str());
}

inline void end_section(const std::string &name) {
ESPRESSO_PROFILER_MARK_BEGIN(name.c_str());
}

The second one should be ESPRESSO_PROFILER_MARK_END. The regression was introduced in 1df1009. When calling these functions inside the body of a Python loop, the Caliper summary creates a recursive data structure for the marked region, since the end marker is missing.

The Cython interface passes Python string objects to C++ functions that take std::string as arguments, triggering runtime exceptions. The interface should use espressomd.utils.to_char_pointer() to handle type conversion. The regression was introduced in df5d9d2.

There are no test cases for this feature.

@jngrad jngrad added the Bug label Jun 1, 2023
@jngrad jngrad added this to the ESPResSo 4.3.0 milestone Jun 1, 2023
@jngrad jngrad self-assigned this Jun 1, 2023
@kodiakhq kodiakhq bot closed this as completed in #4747 Jul 14, 2023
kodiakhq bot added a commit that referenced this issue Jul 14, 2023
Fixes #4740, fixes #2509

Description of changes:
- third-party tools:
   - fix the broken Caliper integration and write an integration test
   - use standard Caliper macros instead of macro aliases
   - build Caliper as a CMake subproject to inherit options (e.g. CUDA and MPI support) and facilitate linking
   - add native support for CUDA-GDB (a few compiler flags were missing from `Debug` builds)
   - add native support for kernprof (new flag added to `pypresso`)
- documentation:
   - centralize scattered information about debuggers into a self-contained section of the user guide
   - document all supported debuggers and profilers: Caliper, Valgrind, GDB, CUDA-GDB, kernprof, perf, sanitizers
   - make the Doxygen docs more consistent with the Sphinx user guide (same fonts) and website (same logo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant