Skip to content

Optimisation: Improve the caching of concentration(time) calls by ensuring that floats are used throughout the cara codebase when representing time

Philip Elson requested to merge pelson/cara:optimisation/better_concentration_caching into master

The key difference here is that we currently have a mix of float, int, and numpy scalars. The latter is the biggest problem, as it isn't hashable and therefore doesn't work well (at all) as a cache key. The ability to pass integers remains, but there are no longer any tests which do so - this isn't such an important detail of this MR, but was extremely helpful to track down the use of numpy scalars.

The result of this change is a significant speed up. On my machine, I see:

Before:

[I 210805 17:27:29 web:2250] 200 GET /calculator/baseline-model/result (::1) 4795.50ms
[I 210805 17:27:34 web:2250] 200 GET /calculator/baseline-model/result (::1) 3360.27ms

After:

[I 210805 17:26:54 web:2250] 200 GET /calculator/baseline-model/result (::1) 3326.71ms
[I 210805 17:27:05 web:2250] 200 GET /calculator/baseline-model/result (::1) 1810.76ms

(note that there is always a penalty for the first result - I'm not sure why tbh.)

You can see that the report is now around 40% quicker to generate. This will go well with the change in !233 (merged) and result in even faster times. Reminder: The actual server that cara is running on is slower than my machine by about a factor of 2, so these numbers should be seen relatively, not absolutely.

Fundamentally the optimisations at play here are:

  • We cache the _concentration_at_state_change result on an instance, not on the class as we did before. This makes calculating the cache key quicker, as it doesn't need to consider the instance as well as the arguments
  • We use the _concentration_at_state_change method inside integrated_concentration for the computed start time. This time isn't always at a state change, but most of the time is.

Merge request reports