-
Notifications
You must be signed in to change notification settings - Fork 311
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
[SofaCore] Add new (simpler) DataEngine implementation #760
[SofaCore] Add new (simpler) DataEngine implementation #760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as it is more the behavior we want, maybe one of the current engine could already become a simpleDataEngine?
As an example and it would go through the CI tests
/// Automatically adds the input fields to the datatracker | ||
void addInput(sofa::core::objectmodel::BaseData* data) | ||
{ | ||
m_dataTracker.trackData(*data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not use to the dataTracker, if the data are added in the dataTracker, is it not possible to use it in the update method to check the data instead of calling the updateIfDirty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The datatracker is not influenced by the updateIfDirty() call. You can absolutely use the datatracker in the update() method if needed (to check which inputs have been changed, and to specific actions for each dirty data. The call to UpdateIfDirty() simply guarantees that all data have been accessed (thus updated upstream) before calling cleanDirty().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so the dataTracker just return dirty if on of its Data is dirty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the dataTracker is not at all used in the internal mechanism of the DataEngine class actually. It's a helper object (one instance per dataEngine) that tracks data. It has a very simple interface, and you can just call
trackData(BaseData*)
<< sets a datafield in its map. for now on, its counter will be tracked
clean() / clean(BaseData*)
<< resets the data's counter
isDirty() : isDirty()
<< checks if the counter is up to date
You can then use this in your update() ( or doUpdate() now) method to see if a specific datafield is dirty or not.
@epernod Good idea. I would suggest the TransformEngine, since it's a very basic engine (the most used also) so the feature would be more visible to users with minimal code change. |
This PR was set to ready too soon. DataEngine_test doesn't compile ^^ |
Waiting for CI but I think it's better now: |
CI was missing disk space. I relaunch ;) |
alright, that should fix the failing test. Although, the test doesn't reflect very well the behavior of the component. the SimpleDataEngine must exclusively be used in cases where the process performed the engine has to be applied no matter which datafield is set to dirty. concretely, it means that in DataEngine_test.cpp, line 151 would assert FALSE |
Following our discussions: void update() final
{
updateAllInputs() // a method that calls `updateIfDirty()` on all inputs: can be overridden, but rarely necessary
DDGNode::cleanDirty() // same as cleanDirty(), but does NOT call m_datatracker->clean() so counters are still valid after
doUpdate() // actual magic
m_datatracker->clean() // cleaning the counters
} To my understanding, DataTrackerDDGNode's I believe there would be no side effects (apart from being BRAKING...) if we would replace |
What about ? void update() final
{
updateAllInputs(); // a method that calls `updateIfDirty()` on all inputs: can be overridden, but rarely necessary
cleanAllInputsDirty(); // same as cleanDirty(), but does NOT call m_datatracker->clean() so counters are still valid after
doUpdate() // actual magic
cleanAllInputsCounter() // cleaning the counters
} |
…:core::DataEngine
I'm rebasing this before merge ;-) |
d313293
to
87dab0b
Compare
Here's a simpler implementation of sofa::core::DataEngine. This implementation hides the code related to updating datafields and calling cleanDirty() in an attempt to reduce human errors.
This PR:
Reviewers will merge only if all these checks are true.