Driver/ami430 allow all units & fix warnings as exceptions

[spauka]

Fixes #952, #953.
In addition, adds a switch heater submodule that allows for setting of SwitchHeater parameters.

Changes proposed in this pull request:

• Allows the AMI430 to be operated with ramp rates specified in minutes, allows field units in kiloGauss.
• Adds an instrument specific exception for instrument errors (i.e. allows us to differentiate why ramps may have failed between validation errors and instrument state)
• Fixes bug in current_ramp_limit code that prevents user from setting a limit higher than 0.6A/s
• Allows user to set field_ramp_limit, field_rating in instrument units, rather than having to convert to Amps.
• Adds AMI430_SwitchHeater submodule that allows configuration of switch heater parameters.
• Removes duplicated code between set_field, _set_field and ramp_to. Makes block a named parameter.
• Deprecate ramp_to as it duplicates the functionality of ramp_to.
• Don't automatically turn on switch heater before ramping, we may have a persistent current and this could cause a quench.
• Ensure the number of segments in ramps is 1. There is currently no support in the driver for more than 1 ramp segment.

API changes

There are no changes to the public facing API apart from the addition of new features and the deprecation warning on ramp_to.

@sohailc I notice that you're also working on changes to how 3D works in this driver, in PR #910. None of the changes as far as I can tell intersect with yours, but as a result I haven't spent much time adding documentation or extending 3D functionality. Can we chat and discuss how best to add docs and type hints such that we don't conflict?

@jenshnielsen @sohailc

[codecov]

Codecov Report

Merging #954 into master will increase coverage by 0.03%.
The diff coverage is 69.92%.

@@            Coverage Diff             @@
##           master     #954      +/-   ##
==========================================
+ Coverage   77.05%   77.08%   +0.03%     
==========================================
  Files          37       37              
  Lines        5182     5233      +51     
==========================================
+ Hits         3993     4034      +41     
- Misses       1189     1199      +10

[sohailc]

I do not quite understand why we need to make a custom exception. Can you explain?

[spauka]

Just so that errors that are specific to the instrument state are differentiated from user errors (like ValueErrors) or python errors. I suppose they can be replaced by RuntimeErrors, but this seems like a poor fit given that they are used everywhere, and the error type suggests the error occured within python.

In particular, I imagined using it to detect a failure to sweep field in my own code, then automatically taking action.

[sohailc]

Yeah I see your point.

[sohailc]

This seems like a good modification to me. I have one comment if you can address that, please. Other than that, good job

[spauka]

Awesome :)

I can't merge, but if you guys are happy can we merge this?

[sohailc]

I think before we can merge we need to make sure all checks pass. It should be quite trivial to make this happen :-)

[WilliamHPNielsen]

@sohailc But the tests are passing? The few issues from Codacy are actually non-issues. See here: pylint-dev/pylint#1609. The lambdas were there before this PR.

[sohailc]

Even though the lambda's are from before this PR, I think we should take every chance we get to improve code quality. @spauka can you please review the code quality suggestions and make improvements accordingly? They won't take more than a couple of minutes.

@WilliamHPNielsen Lets ignore the Codacy issues.

[sohailc]

Oh and obviously please make sure that this branch is up to date with the base branch.

[spauka]

Hey @sohailc. Regarding Codacy issues:

• As @WilliamHPNielsen mentions, unused class variables are not true, both AMI430SwitchHeater and _Decorators are used.
• Regarding lambdas, there isn't an easy way around this. Both get_cmd and set_cmd wrap the argument as a Command, which checks qcodes.utils.deferred_operations.is_function, which returns False for a parameter. Either the behavior of Command is changed, or it has to be wrapped in a function or lambda.

Regarding code coverage, I guess this refers to the new SwitchHeater class. I can add tests tomorrow.

[sohailc]

@spauka I don't understand. Why can you not change this:

get_cmd=lambda: self.current_ramp_limit()

to

get_cmd=self.current_ramp_limit

? Since "current_ramp_limit" is a class method and not a parameter, this should be fine.

[spauka]

@sohailc See the above comment. self.current_ramp_limit is a parameter, and is_function does not detect a Parameter as a function with the correct number of parameters.

[sohailc]

You are right. I will merge this branch now after updating it

[sohailc]

Hang on, the CI tests have failed this time. @spauka can you re-run the test by making a trivial change and pushing your code again?

[spauka]

@sohailc Should be all good :)

[sohailc]

Merged :-)

[WilliamHPNielsen]

@sohailc When the tests fail, you (unfortunately) have to examine why. Travis is very slow at running tests, and a test counts as failed if it exceeds the hypothesis deadline. That puts us in a situation where tests fail not due to problems with the code, but due to heavy load on Travis' servers. It should be pretty easy to deduce from the Travis log what happened.

It is not optimal this way, and we should probably increase/disable deadlines if this keeps happening. @jenshnielsen already did this a few times, see #912 and #905.

[jenshnielsen]

We should probably just disable the hypothesis deadline on those tests