labrad / servers

LabRAD servers
24 stars 21 forks source link

Backwards compatibility broken on old dataset with Units None #149

Closed joshmutus closed 9 years ago

joshmutus commented 9 years ago

When opening an old dataset with the new units branch I get the following traceback: File "data_vault.py", line 504, in getPar data = T.evalLRData(S.get(sec, 'Data', raw=True)) File "/Library/Python/2.7/site-packages/labrad/types.py", line 394, in evalLRData return eval(s) File "", line 1, in

File "/Library/Python/2.7/site-packages/labrad/units.py", line 190, in new raise RuntimeError("Cannot construct WithUnit with unit=None. Use correct units, or use a float") exceptions.RuntimeError: Cannot construct WithUnit with unit=None. Use correct units, or use a float [payload=None]

This dataset contains the parameters [Parameter 14] label = ParampBiasVoltage data = Value(0.90000000000000302, None)

maffoo commented 9 years ago

Oh man, that's bad. Good find. I have been worried about this text-based storage of data values for some time, precisely because it's so brittle and hard to maintain compatibility. In my update to the data vault, I changed the parameter saving to instead save a base64-encoded version of the raw labrad data to avoid this kind of thing (#24). Note that I also added a binary data format in that branch, but that's likely to be superseded by @ejeffrey's work on hdf5 support. However, we may want to strip out the binary data format and merge the rest, to avoid issues like this cropping up in the future. Of course, that won't fix the problem for old data sets, which might require that we manually migrate the old parameters to a compatible format, like Value(0.9, '') or even just 0.9.

ejeffrey commented 9 years ago

Yeah, I am already working on a fix for this in main that will just special case 'None' values and convert them to dimensionless units.

Evan

On Wed, May 13, 2015 at 11:25 AM, Matthew Neeley notifications@github.com wrote:

Oh man, that's bad. Good find. I have been worried about this text-based storage of data values for some time, precisely because it's so brittle and hard to maintain compatibility. In my update to the data vault, I changed the parameter saving to instead save a base64-encoded version of the raw labrad data to avoid this kind of thing (#24 https://github.com/martinisgroup/servers/pull/24). Note that I also added a binary data format in that branch, but that's likely to be superseded by @ejeffrey https://github.com/ejeffrey's work on hdf5 support. However, we may want to strip out the binary data format and merge the rest, to avoid issues like this cropping up in the future. Of course, that won't fix the issue here, which might require that we manually migrate the old parameters to a compatible format, like Value(0.9, '') or even just 0.9.

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/149#issuecomment-101767906 .

joshmutus commented 9 years ago

So I ran the data_vault from Evan's commit and I get a different error now:

Error: (0) [Data Vault] Remote Traceback (most recent call last): File "/Library/Python/2.7/site-packages/labrad/server.py", line 262, in handleRequest result = yield setting.handleRequest(self, c.data, data) File "data_vault.py", line 1050, in open dataset = session.openDataset(name) File "data_vault.py", line 397, in openDataset dataset = Dataset(self, name) File "data_vault.py", line 470, in init self.load() File "data_vault.py", line 514, in load self.parameters = [getPar(i) for i in range(count)] File "data_vault.py", line 505, in getPar data = T.evalLRData(datastr) exceptions.UnboundLocalError: local variable 'datastr' referenced before assignment [payload=None]

maffoo commented 9 years ago

@joshmutus, which commit are you referring to?

joshmutus commented 9 years ago

Sorry, Evan has branch to just test a quick fix for this:

u/ejeffrey/dv_fix_None The commit I'm referring to is: 9ec5ac5ce951dc5d023a52476805c6a1ef985c8b

joshmutus commented 9 years ago

ada7ca1 fixes data_vault.open() but now dv.get() (also dv.get(100), dv.get(100,True)) returns: DimensionlessArray([], shape=(1, 0), dtype=float64)

maffoo commented 9 years ago

How much data is in the data set?

joshmutus commented 9 years ago

Depends on the set, 5 columns, 801 rows is pretty typical

joshmutus commented 9 years ago

Just some context, this is data I just copied from the common data vault and used to set up a data vault on my local computer to test some things, so I can still access all my data at UCSB which as far as I can tell is using an older version of the server.

ejeffrey commented 9 years ago

The main issue seems to be that skynet has the old version of the pylabrad without strict units, and the datavault doesn't work with that, at least for datasets saved previously.

On Wed, May 13, 2015 at 3:27 PM, joshmutus notifications@github.com wrote:

Just some context, this is data I just copied from the common data vault and used to set up a data vault on my local computer to test some things, so I can still access all my data at UCSB which as far as I can tell is using an older version of the server.

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/149#issuecomment-101837611 .

maffoo commented 9 years ago

It's not clear to me why the new data vault with new pylabrad would be giving no data back, however. The change you've made to fix parameter evaluation should not affect that at all.

joshmutus commented 9 years ago

Me neither. So more info: I'm running this on my macbook using the scala labrad manager. Any suggestions to try and debug this? The CSV files, just look like normal CSVs if I open them in a text editor

joshmutus commented 9 years ago

Also, the datavault server spits out this when I try dv.get(): 2015-05-13 15:55:29-0700 [ServerProtocol,client] /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/numpy/lib/npyio.py:809: exceptions.UserWarning: loadtxt: Empty input file: "<open file '/Users/mutus/DataVault/140322.dir/00020 - Power Sweep PNA Scan.csv', mode 'a+' at 0x10dbbbf60>"

ejeffrey commented 9 years ago

OK, this is helpful. It look like np.loadtxt is failing. Lets just look at this tomorrow or something.

Evan

On Wed, May 13, 2015 at 3:56 PM, joshmutus notifications@github.com wrote:

Also, the datavault server spits out this when I try dv.get(): 2015-05-13 15:55:29-0700 [ServerProtocol,client] /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/numpy/lib/npyio.py:809: exceptions.UserWarning: loadtxt: Empty input file: ""

— Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/issues/149#issuecomment-101841567 .

joshmutus commented 9 years ago

So, trying to debug this if I just run numpy.loadtxt on the file I get an error about invalid literal for float. If I add delimiter=',' then it works.

joshmutus commented 9 years ago

Which I now see the loadtxt call in data_vault has delimiter set to ','

maffoo commented 9 years ago

I tried out the data_vault from master and it's broken in a lot of ways. The problem is that since we've been running the multi-headed data vault only, it has been getting the bug fixes and the two have drifted apart. I was aiming to fix this with the binary_data branch by refactoring the two servers to share a common code base, so we don't have to maintain parallel versions. I've stripped out the binary data backend and added the fix for parameter evaluation and pushed the result a u/maffoo/dv_refactor.

joshmutus commented 9 years ago

So should I mark this as resolved or keep it open as a reminder to get the master data_vault working?

joshmutus commented 9 years ago

(btw this works for me, thanks for fixing it!)

maffoo commented 9 years ago

I've created #150 to get the single- and multi-headed code consolidation into master.

joshmutus commented 9 years ago

Cool, thanks