-
Notifications
You must be signed in to change notification settings - Fork 45
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
Bugs with restoring expansion type when runmanager is reopened #50
Comments
Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd). Ran into this bug today. I created 8 globals each storing a list of 20 values and set them from outer to nothing. Everything worked fine until I closed runmanager and reopened it to find my RAM going through the roof. This then resulted in a freeze of our lab machine which could only be resolved by the hard reset button. This should really be fixed. |
Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey). I'm a bit surprised by the PC crash. Do you have an example global file I can reproduce with? As a possible workaround: I'm assuming you removed "outer" because you wanted a list of values in your labscript file? If so, wrapping the list in |
Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington). Runmanager currently computes the number of shots by expanding the globals, so it would have been attempting to create a list of 20^8 = 25 billion elements, so I can see how that would fill system memory. Python should give a MemoryError, but perhaps the computer has swap space that it would have thrashed before then which could have made the computer unusable. The expansion of the shots in |
Original comment by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey). Oh, that explains it! We could fix the first half of this by enforcing that lists/numpy arrays are always treated as either outer or zip (and so expansions can't be empty) and that otherwise you must specify as a tuple or other list like data type. Are there any down sides to that really given we can use tuples for things that shouldn't be expanded? The second half of the issue could be swept under the carpet by asking people to not change zip to outer but to a independently named zip group if they want it to persist across restarts, so that the name is not reverted (aka, leaving it as-is). These changes would save us from having to store additional information in the HDF5 file. Actually: might we be able to revisit the concept of "outer" entirely and replace it with a zip group containing a single global (at least as far as the backend is concerned)? Not sure exactly...but I think we get a few possible options if we enforce that list and numpy arrays must be expanded always. |
Original comment by Chris Billington (Bitbucket: cbillington, GitHub: chrisjbillington). I suspect we can just make runmanager not change the expansion types when it is starting up, can't we? It doesn't change them every other time you change a global, it's just overzealous the first time. guess_expansion_modes is trying to do this correctly:
But there must be a bug in here. |
Original comment by Jan Werkmann (Bitbucket: PhyNerd, GitHub: PhyNerd). Using tuples solved the problem for me. I don't know that they don't expand. I think it might be a good idea to get rid of non expanding lists and arrays. But we should document somewhere, that tuples will not expand and should be used in places where expansion is 'unwanted'. |
Original report (archived issue) by Philip Starkey (Bitbucket: pstarkey, GitHub: philipstarkey).
Following on from issue #49, there are two outstanding bugs I have found that I could not fix. These are:
I think that to fix these we will need to save some data as to whether the expansion type was set manually by the user or automatically by runmanager (and that this needs to be stored in the HDF5 file along side the expansion type). My reasoning for this is that we currently guess the expansion type which is either none or expansion, and then decide if it is a zip group. Because we transition from outer -> zip, we thus can't distinguish zip -> outer as it will appear like it's at the start of the transition from outer -> zip. Similarly, arrays that have had their expansion type removed, will have the expansion type guessed as "outer", since the expansion is determined from the type and so you can't tell the difference.
There may be a way to solve this (without storing extra data in the HDF5 file) by rebuilding the metadata used in the calculation of the expansion type, when runmanager restores open groups. However, I'm not 100% certain this is possible...
The text was updated successfully, but these errors were encountered: