Instrument.close()

[eendebakpt]

The Instrument.close() removes the .name field from the Instrument. This gives problems when printing the instrument. The removal happens in strip_attrs.

I am not sure why all attributes are stripped, but maybe we could make an exception for the .name (and other variables) such that the .__repr__ and other basic functions still work.

Steps to reproduce

1. Create an instrument
2. Call instrument.close()
3. Type print(instrument)

   Expected behaviour

The name of the instrument should be printed, perhaps in combination with a notifier that the instrument is closed

Actual behaviour

An error is generated

@alexcjohnson @giulioungaretti

[eendebakpt]

@alexcjohnson Could you give me an idea why the stripping takes place and it is not sufficient to close the connection and remote the instance ( see https://github.com/qdev-dk/Qcodes/blob/6375f613bb0e80f5ec133c7b83ac7ebb006a7ab6/qcodes/instrument/base.py#L244)? I can make a PR to keep the name intact when stripping

[alexcjohnson]

I am not sure actually how necessary or effective it is, but the goal was to delete references to persistent objects or circular references so that the instrument and its associated resources could be garbage collected. We've had, and I believe continue to have, some difficulty with instruments not releasing their resources when they should.

Anyway I like the idea of whitelisting a few attributes and marking it as closed for repr, thanks!

[giulioungaretti]

I would remove it, if you are neither sure it's necessary nor effective: that is, unless there is a test where the difference it's obvious.

[alexcjohnson]

I would remove it

I wouldn't want to remove it entirely without (manually) testing for example memory usage and resource availability as you create and recreate instruments, restart the notebook kernel... but you're right, it is going to be a little bit tricky to come up with a test case showing that we need this.

[eendebakpt]

I am with alex here: let's not remove it entirely, it is good that a closed instrument cannot be used any more. I made a PR which fixes the issue and adds a test

[giulioungaretti]

Your call, I would not trust any code that does something that it's neither sure necessary nor effective.

[alexcjohnson]

I wouldn't say I trust it... but I trust removing it even less 😱 . Just something to flag for cleanup later.

[giulioungaretti]

lol do I smell a circular referenced danger zone ?

Add just because maybe, remove not because maybe not ?

[alexcjohnson]

💩

There may have been a concrete rationale when I implemented this but I can't say that for sure.

[peendebak]

@alexcjohnson @giulioungaretti @AdriaanRol I have reopened this issue because I have more errors related to the closing of an instrument.

After closing one of my instruments the station.snapshot is triggered which generates an error because the parameters attribute was stripped. I could add parameters to the whitelist as well, but I will probably create a whole cascade of parameters.

Deciding how to solve this one is also related to multiprocessing (closing an instrument should be similar to losing a connection to an instrument server).

[giulioungaretti]

@eendebakpt @peendebak do you have a minimal example so I can look into it ?

[peendebak]

@giulioungaretti Here is a minimal example. I know the station might disappear later, but the principle holds: after closing an instrument any object holding a reference to the instrument and expecting certain attributes or functions will fail.

import qcodes
from qcodes.tests.instrument_mocks import DummyInstrument

x = DummyInstrument(name='test', server_name=None)
d=x.dac1

station=qcodes.Station()
station.add_component(x, 'myinstrument')
station.set_measurement( x.dac1 )
station.snapshot()
station.measure()

print('close the instrument')
x.close() # fine
print(x) # fine
station.snapshot() # fail

[giulioungaretti]

What is the point of stripping the attributes of instrument when closing? Then it's almost the same as deleting it, no ?

I am kind of missing what closing is supposed to mean (except closing the hardware connection).

[peendebak]

I did not write the code, so I cannot say for sure. Let me write what I think should happen:

• After a close the connection to the hardware should be closed. This means that 1) a new connection can be made, 2) even if some other process sends a command to the instrument it should never arrive at the hardware
• Performing a set or similar on the instrument should raise an error or warning
• Performing a get should raise an error or warning, or return the latest value

and now for the tricky part

• Other processes still using the instrument (e.g. monitor, logging, station, etc.) should be able to continue working. E.g.: a monitoring application should not stop or crash or wait indefinitely on a get

The last requirement prevents us from deleting the entire object (I think). I am not sure how to solve this one.

@alexcjohnson

[giulioungaretti]

@eendebakpt / @peendebak so:

close means (in oxford-like English): fucking nuke the entire instrument, gone forever. With the possibility to start anew, which means same hardware but new process new memory and new everything.

And the tricky part should not be tricky, if the instrument is gone an error exception should be raised and handled by other other processes. Which means that the snapshot taken after the instrument is closed will not have the instrument snapshot, and there parameter added to the instrument.