pkpdapp-team / pkpdapp

A web application for modeling the distribution and effects of drugs.
BSD 3-Clause "New" or "Revised" License
9 stars 3 forks source link

bug: endless `simulate` end-point calls if the first simulate fails #401

Closed martinjrobins closed 6 months ago

martinjrobins commented 6 months ago

If there is something wrong with the variables stored in a combined_model (i.e. the name of a variable is inconsistent with the code of that model), then the call to simulate will fail with a 500 errror. If this is the case the front-end will be stuck in an endless loop trying to run a simulation. Need to fail gracefully and stop overloading the server.

martinjrobins commented 6 months ago

you can recreate this by purposefully messing up the model: e.g. change one of the qname values of a stored variable (in the admin interface: http://localhost:8000/admin/) so that it is different from the mmt code for the combined_model. Then open this project in the front-end

martinjrobins commented 6 months ago

update. I have an old project that recreates this problem even if there is no error with simulate. If I select this project first (after a refresh, if I select this project without selecting any others first) then the front-end gets stuck in an endless loop calling simulate and getting back a 200 ok response. Its a fairly normal project, just a three compartment pk model with "other" species. If I create this project again with the new version this is fine, so I can't re-create with another new project sadly

eatyourgreens commented 6 months ago

403 aborts the setState calls that handle the response, if simulate is called while a request is pending. I don't know if that will fix this, but it might help if the problem is an endless loop of request => setState => request => setState etc.

eatyourgreens commented 6 months ago

To reproduce this:

  1. git checkout master
  2. Build a project that runs a simulation.
  3. git checkout development
  4. Select that project when the frontend first loads.
martinjrobins commented 6 months ago

403 went into master, should that have been development?

eatyourgreens commented 6 months ago

I fixed the race condition on both branches, but that turned out not to be the problem at all. #404 fixes this in development, for my local setup at least.

eatyourgreens commented 6 months ago

The bug was a hook that returned [] on each render for projects that have no groups or biomarkers. The new, empty array on each render tiggered re-renders in hooks that depended on subject groups and biomarkers.

The solution is to create a static reference to an empty array, then return that (so that hook returns the same object on each render.)

eatyourgreens commented 6 months ago

Fixed by #404.

eatyourgreens commented 6 months ago

A better solution, long-term, would be to remove the useEffect that wraps the simulate() call. Instead, call simulate directly from event handlers eg. when model parameters change. See You might not need an effect, from the React docs.