Feature/simulated instruments

[WilliamHPNielsen]

For testing purposes, it would be very useful to be able to instantiate drivers connecting to a simulated instrument rather than a real one. No simulation can (reasonably) be expected to perfectly match the behaviour of a real instrument, but this would still be good to have, as simply seeing whether the driver instantiates can be very helpful in the CI process. As an example things like #849 would be caught.

To test the driver code, it is important to leave that code untouched. We should therefore distinguish between a mock driver and a mock (simulated) instrument. Note that we have a single test that already does this (for AMI430), but with a home-made implementation of a mock instrument. This PR suggests using PyVISA-sim (https://pyvisa-sim.readthedocs.io/en/latest/index.html#) to create simulated instruments to communicate with.

PyVISA-sim is the "correct" solution in the sense that it is a part of PyVISA that we are already heavily using. Non-VISA instruments won't work, but we'll be able to write tests for some 80% - 90% of our drivers, which is a lot more than what we currently have. The simulated instruments are just .yaml files, so we may be able to auto-generate them from the QCoDeS parameters in some cases.

NB: The current state of this PR is still very much WIP.

Changes proposed in this pull request:

• Add new sims folder with a simulated instrument (dummy.yaml) and a dummy driver to communicate with it (dummy.py)
• Change the VisaInstrument class to detect that we are using the sim backend
• Add a simulated AMI430 instrument
• Add a dependency-injecting class IPToVisa to allow us to use PyVISA-sim with the AMI430 driver

Still pending:

• Make the simulated AMI430 pass the test written for its driver
• Simulate ALL THE INSTRUMENTS! (deferred)
• Add tests for simulated instruments (deferred)
• Document how to write and add a simulated instrument under the "How to write a driver" documentation
• Make a way to auto-generate a simulated instrument (deferred)
• Get feedback from competent people (you!)

@QCoDeS/core

[WilliamHPNielsen]

@QCoDeS/core, I think this is a good time for review. There is now a PyVISA-sim based test for the AMI430 that (as far as I can tell) carefully reproduces the existing test. This is, too me, proof-of-concept that we can indeed use PyVISA-sim for writing driver tests. And the new test is 5 times faster than the old one! 🎉

I know that this PR got kind of really messy, but just disregard all the validation stuff and my OCD-like pep8'ing. The relevant files are: dummy.yaml, dummy.py, test_ami430_pyvisasim.py, AMI430.yaml, and to some extent AMI430_VISA.py.

[Dominik-Vogel]

We could think about adding an explicit method to determine whether an instrument is simulated while we keep the idn is identical to the non-simulated instrument. This is in any case necessary for drivers that read the idn to determine their model specific behaviour.

[WilliamHPNielsen]

You can determine whether the instrument is simulated by checking what VISA backend you are using. And you are absolutely right, we sometimes need to write several different simulated instruments per driver with different firmware and/or model numbers.

[WilliamHPNielsen]

@QCoDeS/core Okay, save for documentation on how to write simulated instruments, all the above issues should be addressed.

[Dominik-Vogel]

I have an issue with the device clear function. I get a not implemented errror when I call the constructor of the instrument with the default option device_clear=True.
My instrument driver is: (Model_336.py)

class Model_336(VisaInstrument):
    def __init__(self, name, address, **kwargs):
        super().__init__(name, address, **kwargs)
        self.connect_message()

The instrument is: (lakeshore_model336.yaml)

spec: "1.0"
devices:
  device 1:
    eom:
      GPIB INSTR:
        q: "\n"
        r: "\n"
    error: ERROR
    dialogues:
      - q: "*IDN?"
        r: "QCoDeS, m0d3l, 336, 0.0.01"

resources:
  GPIB::9::INSTR:
    device: device 1

and the script is:

import qcodes.instrument.sims as sims

from qcodes.instrument_drivers.Lakeshore.Model_336 import Model_336
# path to the .yaml file containing the simulated instrument
visalib = sims.__file__.replace('__init__.py', 'lakeshore_model336.yaml@sim')

ls = Model_336('lakeshore', 'GPIB::9::65535::INSTR', terminator = '\n', visalib=visalib, device_clear=True)

with the traceback:

Traceback (most recent call last):

  File "<ipython-input-1-71ee8fc820f5>", line 1, in <module>
    runfile('C:/Users/a-dovoge/OneDrive - Microsoft/projects/2017-11-29 Lakeshore/test.py', wdir='C:/Users/a-dovoge/OneDrive - Microsoft/projects/2017-11-29 Lakeshore', post_mortem=True)

  File "C:\Users\a-dovoge\AppData\Local\Continuum\anaconda3\envs\qcodes\lib\site-packages\spyder\utils\site\sitecustomize.py", line 710, in runfile
    execfile(filename, namespace)

  File "C:\Users\a-dovoge\AppData\Local\Continuum\anaconda3\envs\qcodes\lib\site-packages\spyder\utils\site\sitecustomize.py", line 101, in execfile
    exec(compile(f.read(), filename, 'exec'), namespace)

  File "C:/Users/a-dovoge/OneDrive - Microsoft/projects/2017-11-29 Lakeshore/test.py", line 7, in <module>
    ls = Model_336('lakeshore', 'GPIB::9::65535::INSTR', terminator = '\n', visalib=visalib, device_clear=True)

  File "c:\users\a-dovoge\qcodes\qcodes\instrument_drivers\Lakeshore\Model_336.py", line 38, in __init__
    super().__init__(name, address, **kwargs)

  File "c:\users\a-dovoge\qcodes\qcodes\instrument\visa.py", line 81, in __init__
    self.device_clear()

  File "c:\users\a-dovoge\qcodes\qcodes\instrument\visa.py", line 122, in device_clear
    self.visa_handle.clear()

  File "C:\Users\a-dovoge\AppData\Local\Continuum\anaconda3\envs\qcodes\lib\site-packages\pyvisa\resources\resource.py", line 270, in clear
    self.visalib.clear(self.session)

  File "C:\Users\a-dovoge\AppData\Local\Continuum\anaconda3\envs\qcodes\lib\site-packages\pyvisa\highlevel.py", line 442, in clear
    raise NotImplementedError

NotImplementedError

I can look into it with more detail if you @WilliamHPNielsen don't know already what is happening

[WilliamHPNielsen]

@Dominik-Vogel I just know what you already discovered; device_clear=True does not work with the @sim library. Since there is never any reason to clear a simulated instrument, which in the nature of things does not exist yet and thus has nothing to clear, I'd propose to either simply remember to always set device_clear=False for simulated instruments or to bake an if visalib == 'sim' into our visa.py. What do you prefer?

[jenshnielsen]

I think this looks good. Left some small inline comments

[jenshnielsen]

This is not really a great explanation :)

[jenshnielsen]

While you are here could you change this to debug?

[jenshnielsen]

remove?

[jenshnielsen]

remove

[WilliamHPNielsen]

Yes I think we should remove this

[WilliamHPNielsen]

Please remove this

[WilliamHPNielsen]

Now there's even documentation. I'll let @Dominik-Vogel answer and then possibly apply the changes he wants. When that's done, I think we should merge this one.

[sohailc]

So will we require all new drivers submitted to include tests? I think we should

[jenshnielsen]

@sohailc IMHO yes at least for visa instruments

[Dominik-Vogel]

@WilliamHPNielsen regarding the device_clear issue, I would definitely prefer adding a if visalib == 'sim' case, so that writing the tests become simpler.

[Dominik-Vogel]

@sohailc I agree that it would be great having a test for every instrument driver. And possibly this could be something that we do as main developers. But requiring it generally, I think, is a little bit too strict. We don't want to lose contributors by requiring test. Tests in general are, to my experience, something that nobody without a deeper background in programming is familiar with. Bear in mind, that Delft may be exceptional in that way that there are many highly experienced qcodes users.

[WilliamHPNielsen]

@QCoDeS/core I think this is ready now.