Skip to content

Conversation

@lsawade
Copy link
Collaborator

@lsawade lsawade commented Aug 27, 2025

Description

When I was trying out the Kokkos::View extents syntax, I asked claude to create a basic cpp project for me to just play around with the dependencies, and not having to wrestle with specfem compilation.

It ended up creating a core initialization framework to handle initialization and ending of Kokkos.

I created this PR that implements everything. Everything is tested include the Python interface, so no issues there.

Claude's sample implementation (Click me)
namespace specfem {

/**
 * @brief Minimal SPECFEM++ core class for testing dependencies
 * 
 * This class provides a simple interface to test that all major
 * dependencies (Kokkos, YAML-CPP, Boost) are working correctly.
 */
class Core {
public:
    Core();
    ~Core() {if (initialized) finalize()};
    void initialize(int argc, char* argv[]);
    void finalize();
    void print_system_info();
    void run_all_tests(argc, argv);
private:
    bool kokkos_initialized_;
    std::string execution_space_;
};

main function that uses it

int main(int argc, char* argv[]) {
    specfem::Core core;
    
    try {
        core.initialize(argc, argv);
        bool success = core.run_all_tests(argc, argv);

        return success ? 0 : 1;
        
    } catch (const std::exception& e) {
        std::cerr << "Fatal error: " << e.what() << std::endl;
        return 1;
    }
}

What's interesting is that it a allows for a pretty clean single executable without a lot of multiplied code:

running 2d:

specfem 2d -p specfem_config.yaml

running 3d:

specfem 3d -p specfem_config.yaml

Let me know what you think.

Issue Number

If there is an issue created for these changes, link it here

Checklist

Please make sure to check developer documentation on specfem docs.

  • I ran the code through pre-commit to check style
  • THE DOCUMENTATION BUILDS WITHOUT WARNINGS/ERRORS
  • I have added labels to the PR (see right hand side of the PR page)
  • My code passes all the integration tests
  • I have added sufficient unittests to test my changes
  • I have added/updated documentation for the changes I am proposing
  • I have updated CMakeLists to ensure my code builds
  • My code builds across all platforms

@lsawade lsawade requested a review from Rohit-Kakodkar August 27, 2025 19:52
@lsawade
Copy link
Collaborator Author

lsawade commented Aug 27, 2025

If approved, this will need additional file edits for the documentation.

Copy link
Collaborator

@Rohit-Kakodkar Rohit-Kakodkar left a comment

Choose a reason for hiding this comment

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

I am mostly on board with this.

  • I think we need a new enumeration that is different from dimension. How about
namespace specfem {
enum class execution_model {
dim2, dim3, globe, ...
}
}
  • Use std::unique_ptr where you can if the ptr is owned exclusively by core (which I think is the case). Or use std::shared_ptr with weak_ptr which might be appropriate as well. I recomment seeing Cherno's videos on those if you haven't.

@lsawade
Copy link
Collaborator Author

lsawade commented Sep 2, 2025

I am mostly on board with this.

Ok! I'm going to wait for @icui 's review as well before making the requested updates

Use std::unique_ptr where you can if the ptr is owned exclusively by core (which I think is the case). Or use std::shared_ptr with weak_ptr which might be appropriate as well. I recomment seeing Cherno's videos on those if you haven't.

I don't think I watched that one yet, will do!

@lsawade lsawade requested a review from icui September 3, 2025 11:25
@icui
Copy link
Collaborator

icui commented Sep 4, 2025

Looks good to me. Shall we rename it to Context since Core is already a folder that includes everything?

@lsawade lsawade linked an issue Nov 6, 2025 that may be closed by this pull request
7 tasks
@lsawade lsawade changed the title [Proposal] specfem::Core struct for initialization of kokkos, mpi, and handling initialization from python side etc. specfem::Context struct and ContextGuard for initialization of kokkos, mpi, and handling initialization from python side etc. Nov 6, 2025
@lsawade lsawade requested review from Rohit-Kakodkar and icui and removed request for icui November 6, 2025 13:24
@lsawade
Copy link
Collaborator Author

lsawade commented Nov 6, 2025

I still have to update the documentation. But I thought since the updates were larger, I still wait for it.

…rd fully manages the created context. For Python we still create a static instance that is checked against. This should theoretically allow for multiple Kokkoses in one executable.
- put context and execution in deeper namespaces
- updated the original specfem2d and specfem3d executables to
  use ContextGuard. They are kept for compatibility for now.
- add print functions to mpi object
- Updates names of the models
- deprecates enumerations::execution
- adds content to enumerations::simulation
- updates references
- specfem::simulation now contains both context and execute function
- the execute function takes as an argument the 2d and 3d
- calls can be made like so
```cpp
try {
  context = specfem::simulation::contextguard()
  success = specfem::simulation::execute("2d", ...)
```
@lsawade lsawade mentioned this pull request Nov 12, 2025
8 tasks
#include <vector>
#include <yaml-cpp/yaml.h>

namespace specfem {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
namespace specfem {
namespace specfem::simulation {

@lsawade
Copy link
Collaborator Author

lsawade commented Nov 21, 2025

I merged devel into #1416 by accident, so we just gotta wait for that one to merge into here, for the conflicts to be resolved.

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 this pull request may close these issues.

Update #1149 proposal implementation of specfem context

4 participants