CampbellGroup / common

Shared campbell lab code.
GNU Lesser General Public License v3.0
8 stars 5 forks source link

script scanner load experiments from directories #229

Open fanmingyu212 opened 6 years ago

fanmingyu212 commented 6 years ago

Addresses #226 and #228.

List of changes:

How to add experiments based on their directory:

This docstring should be added as the docstring for the experiment module (at the beginning of .py file), but not the docstring for the experiment class. If you already have a docstring for the experiment module, you can add it to your existing docstring.

fanmingyu212 commented 6 years ago

@aransfor @theoriginaljuice @gregllong Could you please review this PR?

aransfor commented 6 years ago

@fanmingyu212 I have some time to look at this today. First impressions:

  1. The refresh button no longer works with the config file (all experiments just disappear)
  2. we run SS in a terminal and I can no longer close out without it hanging
  3. I tried to just run it with the Node on the web browser and that is just hanging
  4. It should auto-create a directory if one does not exist (see datavault for example)

I am sure this is the way to go I am just struggling to get it to run

aransfor commented 6 years ago

@fanmingyu212 I just switched back to master and verified that everything is working. Let me know if you dont see any of this strange hanging behavior and Ill look closer into it

fanmingyu212 commented 6 years ago

@aransfor Issue 1: I added a bug fix for that the refresh button stopped working.

Issue 2, 3: SS should work both from command line and from node. I could not reproduce hanging on our computers, but I found a typo in my code when it tries to read from the config file. This bug is fixed in the latest commit. I tried both loading from the config file and the registry, and both work on our computer. Did you add a directory containing many python modules (e.g. common) in the registry Directories key? If so, SS takes a long time finding experiments in common and looks like hanging. I tested it and it hung for 30 seconds before SS started up. The solution is adding only the experiment folders in registry Directories key, such as ["Qsim.scripts.experiments"].

Issue 4: It now auto creates the directory structure in registry, with a key named Directories which is default to an empty list []. If Directories is an empty list (as default), it reads from the config file. This will allow easier transition to registry config, while maintaining backward compatibility.

Please try pulling latest commits to see if they resolve the issues. If SS is still hanging, could you send the full output when you run it in command line, and the value of registry-Servers-ScriptScanner-Directories key?

aransfor commented 6 years ago

I re-ran the server and it starts but when I run the GUI it gives me the following error

Not connected Unhandled error in Deferred:

Traceback (most recent call last): File "/home/qsimexpcontrol/.virtualenvs/labrad/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1274, in unwindGenerator return _inlineCallbacks(None, gen, Deferred()) File "/home/qsimexpcontrol/.virtualenvs/labrad/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1128, in _inlineCallbacks result = g.send(result) File "script_scanner_gui.py", line 79, in populateExperiments sc = yield self.cxn.get_server('ScriptScanner') File "/home/qsimexpcontrol/.virtualenvs/labrad/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1274, in unwindGenerator return _inlineCallbacks(None, gen, Deferred()) --- --- File "/home/qsimexpcontrol/.virtualenvs/labrad/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 1128, in _inlineCallbacks result = g.send(result) File "/home/qsimexpcontrol/LabRAD/common/lib/clients/connection.py", line 31, in get_server raise Exception("Not connected") exceptions.Exception: Not connected

aransfor commented 6 years ago

and then the server hangs

aransfor commented 6 years ago

@fanmingyu212 I got it to work but it seems I cannot select two directories up? if I set the directory key value to Qsim.scripts it breaks but if I set it to Qsim.scripts.experiments it works, can you try different directory levels on your end?

fanmingyu212 commented 6 years ago

@aransfor I tried including Qsim.scripts in the directories key. It seems like hanging but it is actually loading slowly (Took ~1 min on our computer till it successfully loaded the experiments).

The reason that it loads slowly is that SS attempts to import every python module in the directory and check if the module has a docstring including the script scanner loading information. Importing modules that have many functions that are not enclosed in a class is slow, and there are many such scripts in the Qsim.scripts folder. Therefore, SS just loads very slowly.

I think if you have a specific directory for experiments, it is probably good to include only that directory in the directories key. An alternative solution is to not import the python modules, and try to read python modules as text files and looking for the script scanner header. However, as python gives a clean method to read docstring of a module, I think it is more simple and clear if we use python docstring to get the SS header.

fanmingyu212 commented 6 years ago

@aransfor Could you let me know if it works when you set the path to the experiment folder? If it is only a lag issue, can we merge this PR and raise another issue about it? @theoriginaljuice please also let me know if the changes work for you.

aransfor commented 6 years ago

@fanmingyu212 gonna try and get to it tonight

aransfor commented 6 years ago

@fanmingyu212 when I have the folder pointed at Qsim.scripts.experiments it loads almost instantly however when I point it one folder higher it hangs at ScriptScanner starting... I let it run for 5 full minutes and still hasnt loaded, but it seems like something is wrong since I have only included twice as many files and it is taking many thousands of times longer to load if it will load. I will leave it on over night and try but in the end we will need a faster solution if this is what is going on.

fanmingyu212 commented 6 years ago

@aransfor I think I have figured out why the SS load slowly when the path is set to Qsim.scripts. Multiple files in Qsim.scripts has code not enclosed in a function or class. Importing the module will run those code sequentially. Therefore, it takes a long time for SS to load all of them.

If you add if __name__ == "__main__": before all code not enclosed in a function to prevent loading when importing, the loading speed will be much faster.

I tried adding if __name__ == "__main__": in kittykat.py, plotlorentzian369.py, plotlorentzian935.py, and ticklescan.py, and then I can load all experiments in about 1 second.

I think the solution is either adding if __name__ == "__main__":, or limiting the path to a folder only including the experiments.

jayich commented 6 years ago

@theoriginaljuice can you review?

jayich commented 6 years ago

@fanmingyu212 do you know if this PR solves this issue: https://github.com/CampbellGroup/common/issues/231

fanmingyu212 commented 6 years ago

@jayich this PR does not solve #231.

aransfor commented 6 years ago

@fanmingyu212 I have not forgotten about this its seems to work on my end. I am just very busy right now so be patient I would like to test it further

aransfor commented 6 years ago

@theoriginaljuice can you check that this is backward compatible with your code then we'll go ahead and merge

jayich commented 6 years ago

@gregllong can you please check backwards compatibility as well?

jayich commented 6 years ago

@gregllong and @theoriginaljuice , can you both check if experiments load with this code?

gregllong commented 5 years ago

LGTM

gregllong commented 5 years ago

restarted computer and seems like the refresh button text is gone: scriptscanner

gregllong commented 5 years ago

Okay I guess I missed it. The label was never there. Should we have a refresh textlabel for the refresh button?

aransfor commented 5 years ago

@gregllong it had a logo but it only works in linux. Feel free to find a different solution (like putting boring refresh text lol)

fanmingyu212 commented 5 years ago

@gregllong I think if you use this branch, the text "refresh" would be displayed. Works on our computers running Win10.

capture

fanmingyu212 commented 5 years ago

@theoriginaljuice could you check if your experiments loads with this code?