Base class for submodules: base_step.#154
Closed
djmallum wants to merge 5 commits intoChrismarsh:developfrom
Closed
Base class for submodules: base_step.#154djmallum wants to merge 5 commits intoChrismarsh:developfrom
base_step.#154djmallum wants to merge 5 commits intoChrismarsh:developfrom
Conversation
`base_step` name comes from the idea that submodules are steps in a
module.
Uses CRTP to avoid overhead. Derived classes are defined as follows:
```cpp
template<typename data>
class step : public base_step<step,data>
{
public:
void execute_impl(data& d)
{
// Implementation here
}
}
```
`data` is intended to be the module specific data class derived from
`face_info`.
Derived classses should make use of concepts to enforce getters and
setters on data. Example
```
\#include <concepts>
template<typename T>
concept StepConcept = requires(T& t)
{
{ t.input1() } -> std::floating_point;
{ t.input2() } -> std::integral;
{ t.output1(std::declval<double>()) } -> std::same_as<void>
}
template<StepConcept data>
class step : public base_step<step,data>
{
public:
void execute_impl(data& d)
{
// Implementation here
}
}
```
At the moment, it is recommended to supply everything via functions,
even parameters that are set at run time but never changed.
For parameters that are domain wide one could write a function:
```cpp
const double module::data::foo()
{
static const double result = cfg.get("result");
return result;
}
```
Static ensures that the instance is (a) uniform per instance of `data`,
and (b) set once (thread safe).
It does involve a hidding bool and if check that is evaluated at runtime
on every call.
Parameters that are per-face must have a private variable. Designing
submodules derived from `base_step` with getters and setters provides
flexibility for the writer.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new base class base_step for submodules using the Curiously Recurring Template Pattern (CRTP) to avoid virtual table overhead. The class provides a compile-time polymorphic interface where derived classes implement execute_impl and the base class provides the execute method that dispatches to the derived implementation.
Key changes:
- Introduces CRTP-based
base_stepclass template for building submodules - Uses C++20 concepts to enforce that derived classes implement the required
execute_implmethod - Designed to support a modular architecture where submodules are composed as steps in a processing pipeline
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Lemme tell you... That's neat. Most of these are good suggestions that I can resolve tomorrow. |
Owner
|
I haven't tried it before here. Figured might as well have another hot take. |
**Constness** `GuaranteeImplementExecute` now accepts `const T&` and `Data&` types. This ensures that (a) `T` has a function `void execute_impl` that takes a `Data&` and (b) `T` can be `const` and or a reference. **Concepts** The previous concept was circular: a derived class would need to be declared like: ```cpp template<typename Data> class Derived : public base_step<Derived,Data> ``` But since `Derived` is not fully defined, it cannot satisfy the concept. Instead, this is replaced with a static assert that is only tripped if `execute` is called. It is still tripped at compile time and therefore has the same effect. **Constructors** The constructor is now explicitly declared and is protected.
Contributor
Author
|
Updated |
Open
Contributor
Author
|
Closed and now merged with #155 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New submodule base class
base_stepname comes from the idea that submodules are steps in a module.Uses CRTP to avoid v-table overhead. Derived classes are defined as follows:
datais intended to be the module specific data class derived fromface_info.Derived classses should make use of concepts to enforce getters and setters on data. Example
At the moment, it is recommended to supply everything via functions, even parameters that are set at run time but never changed.
For parameters that are domain wide one could write a function:
Static ensures that the instance is (a) uniform per instance of
data, and (b) set once (thread safe).It does involve a hidden bool and if check that is evaluated at runtime on every call.
Parameters that are per-face must have a private variable. Designing submodules derived from
base_stepwith getters and setters provides flexibility for the writer.Future work
Following this, can merge:
database class. This base class could be combined withface_info. It has three uses: (1) providing a constructor for thedataclass to accept amesh_elem,config_file, andglobalobject. Allows for more detailed control and access to configuration, inputs/outputs, and global variables for submodules. (2) provide a caching system to ensure that inputs and outputs (depends/provides) only dereference themesh_elemobject once per time-step, with minimal overhead by the user and memory savings between timesteps. (3) A flexible and inline-able interface for accessing inputs and setting outputs.base_step