labrad / servers

LabRAD servers
24 stars 21 forks source link

Fixed a unit error in ADR server PID control #340

Closed andrewdunsworth closed 8 years ago

DanielSank commented 8 years ago

Thanks, @andrewdunsworth. Can you add the unit labels to the strings? So, instead of seeing T: 4 we'd see T: 4 K.

zchen088 commented 8 years ago

@DanielSank do the severs tests usually pass? They seem unrelated to the change Andrew made.

DanielSank commented 8 years ago

@zchen088 No idea. Check the error on TeamCity?

andrewdunsworth commented 8 years ago

Done. What check did I fail here?

andrewdunsworth commented 8 years ago

What is dev.write doing?

DanielSank commented 8 years ago

dev.write tells the GPIB server to send a GPIB command over the wire.

andrewdunsworth commented 8 years ago

Better?

DanielSank commented 8 years ago

@andrewdunsworth yeah it's better to use format. Nice.

I was wondering if you could put the units in the print statements too.

andrewdunsworth commented 8 years ago

I think the '{}'.format() will do that.

zchen088 commented 8 years ago

@DanielSank

In [42]: '{}'.format(1*ns)
Out[42]: '1.0 ns'
zchen088 commented 8 years ago

@maffoo can you provide any insight on the failed tests?

DanielSank commented 8 years ago

LGTM but someone should go to TeamCity and see why the tests are failing.

andrewdunsworth commented 8 years ago

I don't know what my user name or password is for that? It's not the same as github...

zchen088 commented 8 years ago

I looked, it seems like the datavault is down for some reason?

jwenner commented 8 years ago

@maffoo, could these test failures be due to #335 not being merged in yet?

jwenner commented 8 years ago

There are some additional failures about the first argument to servers in fpgalib.test.test_dac.top_level_collect, fpgalib.test.test_fpga_server.top_level_collect. An example is below:

('fpgalib/test/test_dac.py', None, 'fpgalib/test/test_dac.py')
servers/fpgalib/test/test_dac.py:5: in <module>
    import fpgalib.dac as dac
servers/fpgalib/dac.py:29: in <module>
    import fpgalib.fpga as fpga
servers/fpgalib/fpga.py:5: in <module>
    from labrad.devices import DeviceWrapper
pylabrad/labrad/devices.py:91: in <module>
    class DeviceServer(LabradServer):
pylabrad/labrad/devices.py:313: in DeviceServer
    def select_device(self, c, key=0):
pylabrad/labrad/decorators.py:44: in decorator
    return Setting(func, lr_ID, lr_name, returns, unflatten, **params)
pylabrad/labrad/decorators.py:131: in __init__
    "unless other arguments are optional".format(args[0]))
E   ValueError: First argument key cannot accept '' unless other arguments are optional
ejeffrey commented 8 years ago

Hi Jim,

This is a change in the setting decorator that looks like it has a bug. I am looking into it.

On Mon, Apr 18, 2016 at 2:59 PM, Jim Wenner notifications@github.com wrote:

There are some additional failures about the first argument to servers in fpgalib.test.test_dac.top_level_collect, fpgalib.test.test_fpga_server.top_level_collect. An example is below:

('fpgalib/test/test_dac.py', None, 'fpgalib/test/test_dac.py') servers/fpgalib/test/test_dac.py:5: in import fpgalib.dac as dac servers/fpgalib/dac.py:29: in import fpgalib.fpga as fpga servers/fpgalib/fpga.py:5: in from labrad.devices import DeviceWrapper pylabrad/labrad/devices.py:91: in class DeviceServer(LabradServer): pylabrad/labrad/devices.py:313: in DeviceServer def select_device(self, c, key=0): pylabrad/labrad/decorators.py:44: in decorator return Setting(func, lr_ID, lr_name, returns, unflatten, params) pylabrad/labrad/decorators.py:131: in init** "unless other arguments are optional".format(args[0])) E ValueError: First argument key cannot accept '' unless other arguments are optional

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/pull/340#issuecomment-211599067

ejeffrey commented 8 years ago

@jwenner, @maffoo

decorators,py, the if statement on line 129 should be either 'Nrequired >1' or 'Nrequired != 0'. Can you guys figure out which one and update it?

On Mon, Apr 18, 2016 at 3:17 PM, Evan Jeffrey hobbes@permutationcity.net wrote:

Hi Jim,

This is a change in the setting decorator that looks like it has a bug. I am looking into it.

On Mon, Apr 18, 2016 at 2:59 PM, Jim Wenner notifications@github.com wrote:

There are some additional failures about the first argument to servers in fpgalib.test.test_dac.top_level_collect, fpgalib.test.test_fpga_server.top_level_collect. An example is below:

('fpgalib/test/test_dac.py', None, 'fpgalib/test/test_dac.py') servers/fpgalib/test/test_dac.py:5: in import fpgalib.dac as dac servers/fpgalib/dac.py:29: in import fpgalib.fpga as fpga servers/fpgalib/fpga.py:5: in from labrad.devices import DeviceWrapper pylabrad/labrad/devices.py:91: in class DeviceServer(LabradServer): pylabrad/labrad/devices.py:313: in DeviceServer def select_device(self, c, key=0): pylabrad/labrad/decorators.py:44: in decorator return Setting(func, lr_ID, lr_name, returns, unflatten, params) pylabrad/labrad/decorators.py:131: in init** "unless other arguments are optional".format(args[0])) E ValueError: First argument key cannot accept '' unless other arguments are optional

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/martinisgroup/servers/pull/340#issuecomment-211599067

pomalley commented 8 years ago

@andrewdunsworth how well does the PID work? The fact that this units error is still in there makes me realize just how long it has been since I last even tried the PID functionality...

andrewdunsworth commented 8 years ago

It works for what I want it to do now, mag up and down for field cools.

andrewdunsworth commented 8 years ago

So anyways, do I merge this now?

jwenner commented 8 years ago

I just did a fake PR (#341) which only contains a change to the README in order to determine if the test failures are here or in some more fundamental level. That PR has the same test failures, so my inclination would be to ignore the test failures on this PR but raise them as separate issue(s).