[ISSUE 378] Move Recorder allocation to setup() and delegate via OperationManager.#945
Open
CodersRepo wants to merge 1 commit into
Open
Conversation
Renames init() to setup() on Recorder implementations so memory allocation follows the same lifecycle pattern as Layout and Connections.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the recorder lifecycle in the simulator startup path by renaming Recorder::init() to Recorder::setup() and invoking recorder setup via OperationManager from Model::setupSim(), aligning recorders with the existing “constructor registers operations / setup allocates resources” pattern.
Changes:
- Renamed recorder initialization hook from
init()tosetup()across theRecorderinterface and implementations (XML/HDF5). - Registered
Operations::setupforXmlRecorderandHdf5Recorder, and updatedModel::setupSim()to executeOperations::setupviaOperationManager. - Updated unit tests to call
setup()instead ofinit().
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| Testing/UnitTesting/XmlRecorderTests.cpp | Renames test and updates calls from init() to setup(). |
| Testing/UnitTesting/Hdf5RecorderTests.cpp | Updates calls from init() to setup() and related test comments. |
| Simulator/Recorders/XmlRecorder.h | Updates recorder lifecycle method declaration and documentation to setup(). |
| Simulator/Recorders/XmlRecorder.cpp | Registers Operations::setup and renames implementation from init() to setup(). |
| Simulator/Recorders/Recorder.h | Renames the core recorder lifecycle API from init() to setup(). |
| Simulator/Recorders/Hdf5Recorder.h | Updates lifecycle method declaration and documentation to setup(). |
| Simulator/Recorders/Hdf5Recorder.cpp | Registers Operations::setup and renames implementation from init() to setup(). |
| Simulator/Core/OperationManager.cpp | Adds string mapping for Operations::setup. |
| Simulator/Core/Model.cpp | Executes Operations::setup during model setup instead of calling recorder_->init() directly. |
Comment on lines
23
to
+27
| function<void()> printParametersFunc = std::bind(&XmlRecorder::printParameters, this); | ||
| OperationManager::getInstance().registerOperation(Operations::printParameters, | ||
| printParametersFunc); | ||
| function<void()> setupFunc = std::bind(&XmlRecorder::setup, this); | ||
| OperationManager::getInstance().registerOperation(Operations::setup, setupFunc); |
Comment on lines
25
to
+30
| // Register the printParameters function with the OperationManager | ||
| function<void()> printParametersFunc = std::bind(&Hdf5Recorder::printParameters, this); | ||
| OperationManager::getInstance().registerOperation(Operations::printParameters, | ||
| printParametersFunc); | ||
| function<void()> setupFunc = std::bind(&Hdf5Recorder::setup, this); | ||
| OperationManager::getInstance().registerOperation(Operations::setup, setupFunc); |
Comment on lines
+31
to
+33
| /// Setup the internal structure of the class (allocate memories and initialize them). | ||
| /// Registered to OperationManager as Operation::setup | ||
| void XmlRecorder::setup() |
Comment on lines
+45
to
+47
| /// Setup the internal structure of the class (allocate memories and initialize them). | ||
| /// Registered to OperationManager as Operation::setup | ||
| virtual void setup() override; |
Comment on lines
+39
to
+41
| /// Setup the internal structure of the class (allocate memories and initialize them). | ||
| /// Registered to OperationManager as Operation::setup | ||
| virtual void setup() override; |
Comment on lines
+36
to
+37
| /// Setup the internal structure of the class (allocate memories and initialize them). | ||
| virtual void setup() = 0; |
Comment on lines
+19
to
20
| // Test case for setup() and term() | ||
| TEST(Hdf5RecorderTest, Hdf5InitAndTermTest) |
| recorder.init(); | ||
| recorder.setup(); | ||
|
|
||
| // Ensure the file has been created successfully by the constructor |
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.
Renames init() to setup() on Recorder implementations so memory allocation follows the same lifecycle pattern as Layout and Connections.
Closes #378
Description
enamed Recorder::init() to setup() so Recorders follow the same lifecycle as Layout and Connections: constructors only register operations, and file/output allocation happens in setup().
XmlRecorder and Hdf5Recorder now register Operations::setup with OperationManager, and Model::setupSim() calls that instead of recorder_->init() directly. Unit tests were updated to match.
Checklist (Mandatory for new features)
Testing (Mandatory for all changes)
test-medium-connected.xmlPassedtest-large-long.xmlPassed