Skip to content
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

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

Closed
NeZanyat opened this issue Apr 10, 2019 · 5 comments
Closed

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

NeZanyat opened this issue Apr 10, 2019 · 5 comments

Comments

@NeZanyat
Copy link
Contributor

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
Copy link

gidili commented Apr 11, 2019

Hey @rgerkin any idea how we can fix this?

@rgerkin
Copy link
Contributor

rgerkin commented Apr 12, 2019

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

@NeZanyat
Copy link
Contributor Author

I updated code, but problem still here.

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

@rgerkin
Copy link
Contributor

rgerkin commented Apr 14, 2019

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:

  1. 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
Copy link
Contributor Author

Solution with custom backend works fine, thank you

@rgerkin rgerkin closed this as completed Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants