Ats9870 dev branch #66
Conversation
…he code and structure is written. More functions would be nice and several todo items are important before actual implementation
fixed some small errors
…tors to validate that the value is in the values of the dictionary. The driver now calls the validate function of the parameter class to enforce this validation. Solved the problem with sample rates changed the default of some parameters implemented trivial dictionaries. Driver still largely untested
|
Awesome! 👍 |
| from qcodes.instrument.parameter import Parameter | ||
| from qcodes.utils import validators | ||
|
|
||
| # TODO (C) logging |
There was a problem hiding this comment.
what are (C), (W) etc?
There was a problem hiding this comment.
M o S C o W prioritizes the todo items and stand for respectively:
Must be done, Should be done, Could be done and Would be nice if done.
It is not perfect, but it helps picking out the most important stuff to work on
|
I posted two comments already (see above), but I also see some very good suggestions (I like the abbreviation of setting parameters) so I see there is a little work to be done. I'll report back. |
| self.parameters['sample_rate']._get_byte(), | ||
| self.parameters['clock_edge']._get_byte(), | ||
| self.parameters['decimation']._get_byte()) | ||
| self._result_handler(error_code=return_code, error_source="AlazarSetCaptureClock") |
There was a problem hiding this comment.
this pattern:
return_code = self._ATS9870_dll.Alazarxxxxx(self._handle, ...)
self._result_handler(error_code=return_code, error_source='Alazarxxxxx')gets a little tedious. What about something like this instead of _result_handler:
def call_dll(self, func_name, *args):
func = getattr(self._ATS9870_dll, func_name)
return_code = func(self._handle, *args)
if error_code != self._success:
# TODO (C) log error
# include the args (the beginning anyway) so you don't need the special case
# of one or more args in the error_source, and you get more info to debug with
argrepr = repr(args)
if len(argrepr) > 100:
argrepr = argrepr[:96] + '...)'
# put error_code in a class attribute so we're not constantly re-evaluating the dict
if error_code not in self._error_codes:
raise KeyError('unknown error {} from function {} with args: {}'.format(
error_code, func_name, argrepr))
raise Exception('error {}: {} from function {} with args: {}'.format(
error_code, self._error_codes[error_code], func_name, argrepr))I could even imagine including a kwarg updates=['clock_source', 'sample_rate', 'clock_edge', 'decimation'] that would create the _set_updated() calls that also consistently trail this pattern.
Also, it's maybe a matter of style, but I wouldn't call the unknown error a KeyError because the error itself isn't that the key is missing, that's just the reason you don't have more info to give the user about it. I'd probably call them both RuntimeError
|
@damazter , about a week ago you showed me an example of using the driver with an "acquisition controller". I want to adopt this paradigm when using the ATS and replace our higher level meta-instruments with these "acquisition controllers" (and convince @cdickel and @elrama- that it's a good idea :) ) . I think it is a rather powerful paradigm but I find it hard to reproduce the example from memory; did you hand the controller to the ATS or the other way round? and what are the commands you execute to start an acquisition? I guess besides the docs/examples the acquisition controller docstring itself is a good place for this |
|
I guess that is a good idea but I think the settings of both time domain and frequency domain measurements should be stored somewhere permanently such that one can switch quickly between the two. That was the reason for building the abstraction layer. From: Adriaan [[email protected]] @damazterhttps://github.com/damazter , about a week ago you showed me an example of using the driver with an "acquisition controller". I want to adopt this paradigm when using the ATS and replace our higher level meta-instruments with these "acquisition controllers" (and convince @cdickelhttps://github.com/cdickel and @elrama-https://github.com/elrama- that it's a good idea :) ) . I think it is a rather powerful paradigm but I find it hard to reproduce the example from memory; did you hand the controller to the ATS or the other way round? and what are the commands you execute to start an acquisition? I guess besides the docs/examples the acquisition controller docstring itself is a good place for this � |
The extra kwarg added in this pull request breaks qcodes unless the base instrument class is updated as well to accomodate for this extra kwarg
fixed them to comply with the ATS-SDK programmer's guide
included many more numbers into the idn of the alazar cards
…erence Suggest/add instrumentserver reference
add [parameter]._save_val to the _set method of the alazarparameter to obtain proper snapshots of the alazarparameters
The ats example notebook now works and is a bit more to the point. Comments will be added later
fix comments and it now works with loops
wrote more comments for the acquisition controller. acquisitioncontroller now accepts name instead of id as a reference for the alazar instrument. removed a useless statement passing the alazar reference around
|
@alexcjohnson @giulioungaretti I believe this is it, |
|
I would vote to dance ASAP so that there is a stable version in the master on which @elrama- @nataliejpg and @nulinspiratie can base their acquisition controllers and drivers for other types. I am in no position to judge the PR though, what I understand of it is beautiful and well thought out but I'll leave this to @giulioungaretti |
|
Awesome to hear! I will add the pr for four channels soon. Is it now also On Aug 16, 2016 10:32 PM, "damazter" [email protected] wrote:
|
I made an example of a working loop-acquisition in the example notebook (docs/examples/Qcodes example ATS_ONWORK.ipynb) |
|
@damazter: this looks great, especially <3 the added comments and example notebook. I was wondering why it is that the part where you add the "acquisition" parameter to the acquisition controller (and also of course the 'do_acquisition' function) in the DFT_acquisition_controller isn't part of the base AcquisitionController in ATS.py? Is there some reason why that wouldn't work because to me it seems like a more natural place for them to be? |
|
@nataliejpg Secondly, the do_acquisition function adds the kwargs to the acquire function of the ATS driver. The way you would like to do this also hugely differs from experiment to experiment. For a example controller, it's fine to specify everything explicitely, but for a more integrated setup, some automation in these variables might be nice. Again, these kind of automations are too specific to put in the general driver, so the acquisitioncontroller implementations looked like a nice spot to put them to me. Let me know if you agree. I am very open to hear ideas for improvements, but this is the best that I could come up with. |
|
@damazter oh yes I see, I didnt think about the different forms the "acquisition" parameter would need to have, probably in many cases it will need to be a 'hard' parameter and that will have to be user defined also. I think that part might be one of the more important parts of the acquisition controller to document though seeing as it is becoming less clear as to what is calling what between the alazar card and the acquisition controller. My understanding is that previously you would write a controller which was then used when you called the alazar aquitre function and sorted out the data handling/processing part but on the face of it you would only really interact with the alazar in your notebook (or wherever you run qcodes) and ask it directly to carry out the aquire funtion and it would return the processed data. Now if you want to pull some data you get the "acquisition" parameter of your acquisition_controller which causes the alazar's aquire funtion to be carried out which in turn uses the acquisition controller for its data handling/processing. Does that sound right? I think it's an extra layer with the aim of enabling use of a qcodes paramer. The fact that the qcodes parameter is a parameter of the aqcuisition controller is what adds this extra part. That's not to say that I disagree because I am really keen on having the user only ever need to change the acquisition controller based on what they want to acquire but I do think that this structure needs some explaining in comments/docs as it's no longer really obvious what's going on just from looking at the acquisition controller which would be the ideal. Sorry this got so long. Also this is so far just speculation from reading the code, I'll try actually using it next week and maybe have more useful feedback. |
This is exactly right
yes
I don't follow this sentence :(
More docs would be better, but I am not entirely sure what kind of doc-structure would fit this |
If allocated_buffers > buffers_per_acquisition, more results would be returned than acquired which is not wanted. This condition is now explicitely checked.
|
@nulinspiratie |
|
@alexcjohnson @giulioungaretti any updates on this? It seems to me it is awaiting feedback before it can 💃 |
|
superseded by #338 |
This is version 0.1 of the Alazar driver. It has been tested in so far that we know that it works, but a more complete test set should be included in later versions. In summary, this driver works, but this is not the final version. However people can already start using it, so I think it therefore should be pulled into the master branch of qcodes such that the code is available for everyone.
There are some todo items left in the code which also will get resolved in later versions.