Extended Amici history #1263
Extended Amici history #1263
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1263 +/- ##
===========================================
+ Coverage 84.33% 84.39% +0.06%
===========================================
Files 151 152 +1
Lines 12333 12420 +87
===========================================
+ Hits 10401 10482 +81
- Misses 1932 1938 +6 ☔ View full report in Codecov by Sentry. |
| @staticmethod | ||
| def _simulation_to_values(x, result, used_time): | ||
| values = Hdf5History._simulation_to_values(x, result, used_time) |
There was a problem hiding this comment.
Here it's @staticmethod, but n CsvAmiciHistory it's super().... Just curious, why?
There was a problem hiding this comment.
In CsvHistory it's not static as it uses dynamically updated attributes (e.g. _n_fval), in Hdf5History it's not the case, so I made the methods static both in Hdf5History and Hdf5AmiciHistory.
pypesto/history/amici.py
Outdated
| def _trace_columns(self) -> list[tuple]: | ||
| columns = super()._trace_columns() | ||
| return columns + [ | ||
| (c, np.nan) | ||
| for c in [ | ||
| CPU_TIME_TOTAL, | ||
| PREEQ_CPU_TIME, | ||
| PREEQ_CPU_TIME_B, | ||
| POSTEQ_CPU_TIME, | ||
| POSTEQ_CPU_TIME_B | ||
| ] | ||
| ] |
There was a problem hiding this comment.
Should this also exist for Hdf5AmiciHistory?
There was a problem hiding this comment.
This is for _init_trace which is only needed in Csv case, I think
plakrisenko
requested review from
PaulJonasJost,
m-philipps and
a team
as code owners
Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com>
Co-authored-by: Daniel Weindl <dweindl@users.noreply.github.com>
Co-authored-by: Fabian Fröhlich <fabian.frohlich@crick.ac.uk>
| assert np.all( | ||
| history.get_cpu_time_total_trace() | ||
| >= history.get_preeq_time_trace() | ||
| ) | ||
| assert np.all( | ||
| history.get_cpu_time_total_trace() | ||
| >= history.get_preeq_timeB_trace() | ||
| ) | ||
| assert np.all( | ||
| history.get_cpu_time_total_trace() | ||
| >= history.get_posteq_time_trace() | ||
| ) | ||
| assert np.all( | ||
| history.get_cpu_time_total_trace() | ||
| >= history.get_posteq_timeB_trace() | ||
| ) |
There was a problem hiding this comment.
Wouldn't a better assertion actually be to assert the sum over the different traces to be smaller equal than the total time?
No description provided.