Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1072 +/- ##
==========================================
+ Coverage 96.59% 96.84% +0.25%
==========================================
Files 137 140 +3
Lines 11057 11363 +306
==========================================
+ Hits 10680 11005 +325
+ Misses 377 358 -19 ☔ View full report in Codecov by Sentry. |
Add and use mio::apply with a tuple of results in auto_deserialize_impl.
|
In the last commits I added
I will undo these changes in the next commit. |
dabele
left a comment
There was a problem hiding this comment.
The Members syntax feels much better than the old one i think 👍
Some small problems, and the renaming of auto serialize left.
update documentation per review suggestions
|
On this branch: On main: Benchmarks are made on the Team Server. I would say this looks like no significant change. |
|
I also wrote a makeshift benchmark to compare the TimeSeriesFunctor with the std::function set to [](ScalarType t) -> ScalarType {
return mio::linear_interpolation_of_data_set<ScalarType, ScalarType>({... data ...}, t);
};that was used as a ABM parameter prior to this branch, since abm_benchmark does not set it to anything other than zero. The results are with
Looks like the functor is actually more performant than the std::function, which is a nice plus. Only Zero case could be improved for the functor, but it is probably fast enough. UpdateThe current non-lazy version performs as shown in the table below. Not impressive, but still fast enough. |
DavidKerkmann
left a comment
There was a problem hiding this comment.
Looks great! I have not checked the internal logic for the automatic (de)serialization, bu t I believe you and Daniel that this was checked and iterated.
So in order to use the auto-serialization, one has a couple of options (as one needs the default constructor). You have explained before, but it would be good to go over it again with everyone I think.
Not many remarks, mostly discussion points and typos/comments.
A couple of lines are not covered by tests, perhaps check or we can discuss.
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
If need be, add additional information and what the reviewer should look out for in particular:
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)
Closes #652