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

Save States Patches + Misc. #153

Merged
merged 24 commits into from
Jul 5, 2022
Merged

Save States Patches + Misc. #153

merged 24 commits into from
Jul 5, 2022

Conversation

matt-wolff
Copy link
Contributor

@matt-wolff matt-wolff commented May 23, 2022

  • Changed how save_times in ecoli configs is loaded into a simulation so that no duplicate states are saved.
  • Fixed a bug in saving colony states where processes were being serialized when they weren't supposed to be.
  • Saving the state of a single ecoli cell now uses a condition when calling get_value().
  • Renamed various keys used to access dictionaries, as the names of the actual keys in the dictionaries have changed due to other PRs.
  • Regenerated all vivarium and colony save states.
  • Included saved state time in random seed when loading saved state
  • Avoid serializing Unum objects in saved state by stripping Unum units before emitting criticalMassPerOriC
  • Make sure requesters and evolvers start running when loading a saved state, even when evolvers_ran and Evolver.process.request_set are both false

@matt-wolff matt-wolff requested review from U8NWXD and eagmon May 23, 2022 19:38
@eagmon
Copy link
Contributor

eagmon commented May 23, 2022

Hmm, it isn't clear why pytest failed. Maybe try to re-run?

ecoli/analysis/tablereader.py .                                          [  1%]
[14](https://github.com/CovertLab/vivarium-ecoli/runs/6562218686?check_suite_focus=true#step:8:15)
ecoli/composites/chemotaxis_minimal.py .                                 [  2%]
[15](https://github.com/CovertLab/vivarium-ecoli/runs/6562218686?check_suite_focus=true#step:8:16)
/home/runner/work/_temp/09c4b6ed-cad0-4fa0-9b61-7fb525f1b263.sh: line 1:  2276 Killed                  pytest --cov=ecoli -m 'not (noci or master)'
[16](https://github.com/CovertLab/vivarium-ecoli/runs/6562218686?check_suite_focus=true#step:8:17)
ecoli/composites/ecoli_master_tests.py .
[17](https://github.com/CovertLab/vivarium-ecoli/runs/6562218686?check_suite_focus=true#step:8:18)
Error: Process completed with exit code 137.

@@ -149,7 +149,7 @@ def __init__(self, parameters=None):
'jitter_force': jitter_force,
'bounds': remove_units(self.bounds),
'barriers': self.mother_machine,
'physics_dt': self.parameters['time_step'] / 10,
'physics_dt': self.parameters['timestep'] / 10,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, sorry for changing the parameter name. It's for the greater good!

Copy link
Contributor

@eagmon eagmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll re-run the pytest -- looks like it was killed for some unknown reason.

@U8NWXD
Copy link
Member

U8NWXD commented May 23, 2022

2276 Killed almost always means that the tests ran out of memory and were killed by the operating system

@eagmon
Copy link
Contributor

eagmon commented May 24, 2022

@matt-wolff any new tests added that might be taking a lot of memory?

@U8NWXD
Copy link
Member

U8NWXD commented May 24, 2022

I tried starting a sim from a saved state after merging fce79ed into master, upgrading vivarium-core, and fixing the time_step vs timestep issue, and it looks like something's wrong with the processes in the sim starting from the saved state:

a43b840e-dae2-11ec-9930-837301407705_multigen

U8NWXD added 7 commits May 24, 2022 14:12
When we load a saved state, we have to be sure we don't use the same
seed that we used to generate the state. Otherwise, the unique molecule
IDs will clash. We avoid this by adding the time at which the state was
saved to the seed.
In the saved state, `evolvers_ran` might be false. In that case, since
`Evolver.process.request_set` is also false, we can get an endless loop
of requesters and evolvers waiting for the other to run. To break this
loop, we run the requesters whenever `request_set` is false.
When loading a saved colony state, we were failing to correctly
initialize the exchange data. In the process of fixing that, we also
had to move where we deserialize the saved state to deserialize before
setting the exchange data because the exchange data contains a set,
which our serializers don't understand.
Requires PR vivarium-collective/vivarium-core#198, which introduces
process commands. The process commands interface lets us retrieve
internal state from parallelized EngineProcess instances.
@U8NWXD
Copy link
Member

U8NWXD commented Jun 13, 2022

I just pushed a commit that requires vivarium-collective/vivarium-core#198, so I expect the tests to fail

U8NWXD added 5 commits June 16, 2022 12:47
Upgrade fixes a bug where arrow would hang.
In 492dc42, we solved the problem of neither Evolvers nor Requesters
running by running calculate_request() an extra time in the Evolver.
However, this meant that a request was generated without the Allocator
running, which caused the simulation to crash. Instead, this commit
fixes the problem by ensuring that evolvers_ran is not saved when saving
states.
@U8NWXD
Copy link
Member

U8NWXD commented Jun 21, 2022

Holding off on merging this to avoid changing the codebase too much while @thalassemia is getting started

@U8NWXD U8NWXD merged commit 0e32993 into master Jul 5, 2022
@U8NWXD U8NWXD deleted the save_patches branch July 5, 2022 22:33
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

Successfully merging this pull request may close these issues.

3 participants