scidash / neuronunit

A package for data-driven validation of neuron and ion channel models using SciUnit
http://neuronunit.scidash.org
38 stars 24 forks source link

Can't get values from memory_cache because hash was changed #208

Closed NeZanyat closed 5 years ago

NeZanyat commented 5 years ago

Branch: dash

I'm setting memory cache, and then run judge on model. But instead getting results from memory cache model tries to resolve it with backend. It happens because when I setting cache model sets it under one hash. But then in process of judging it starts setting some params and hash is changing, and model can't find values from cache, because it has another hash now.

Flow:

https://fex.net/s/ponye3o

gidili commented 5 years ago

Hey @rgerkin any idea how we can fix this?

rgerkin commented 5 years ago

Yes, I just fixed that in 9349dd9c835fe6a15f6dc3aaa24b52b0583e7f86 and it is demonstrated here

NeZanyat commented 5 years ago

I updated code, but problem still here.

https://fex.net/s/68myaxy

rgerkin commented 5 years ago

I think you are pointing out in that screencast that during test.get_prediction(), the calling of model.set_run_params() causes the hash to change. This is expected behavior, since any call to set_run_params() could change run parameters that would result in the simulation having a different expected result (and thus it should get/set a different cache).

I see three possible solutions, so let's ask @gidili which makes more sense given other design concerns.

1a) Don't set the cache outside of the test.judge()call, but to set it in your backend's implementation of Backend._backend_run() (defined here), which is where simulator calls would normally occur, and occurs later than all commands that could modify the model's hash. You would just have that method return the dictionary of simulation results previously obtained from Gepetto. Since this is normally the most computationally intensive step, you don't have to worry about computation time for other things occurring inside test.judge(). It would look something like:

class GepettoWebBackend(sciunit.models.backends.Backend):
  backend = 'GepettoWeb'
  def _backend_run(self):
    return YOUR_DICTIONARY_OF_RESULTS_FROM_GEPETTO_SERVER

and could be used with model = ReducedModel(LEMS_FILE_NAME, backend = 'GepettoWeb')

1b) Same as 1a, but for Backend.backend_run() instead of Backend._backend_run(), so you can override the caching logic yourself.

The problem with both of the above, I think, is that @gidili intended for test.judge() to not operate concurrently with any Gepetto simulations. But it doesn't need to, since if you were already planning to write the results of the simulation to the cache before even calling test.judge(), then you must know enough from the instantiated model and test to run the simulation on the Gepetto server, store the results somewhere, and then when test.judge() is eventually called you can have _backend_run() or backend_run() just return those results from memory or disk. If there is any problem with that approach, we could try:

2) I make sure to separate out anything that writes to model attributes (probably nothing) or model run parameters (probably lots of things) into a separate test.condition_model() method that runs before the rest of test.generate_prediction(). Then you would just need to call test.condition_model() before writing to the cache, ensuring that when test.judge() and ultimately test.generate_prediction() are called, the model will have the same hash when it enters those functions as it does when backend_run() checks for that hash in the cache.

I should probably be doing #2 anyway, so I think this could be combined with #1a or #1b. #1a and #1b are more in keeping with the way that other backends are used. Also there really should be backend for this Gepetto simulation bridge, that way in the future it would be possible to have totally matching versions of simulations done on the website and simulations done in notebooks or in the shell. And at the very least there should be a GepettoWebBackend which does absolutely nothing, and which is set when the model is instantiated.

NeZanyat commented 5 years ago

Solution with custom backend works fine, thank you