Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend QCoDeS-Station to be configurable through yaml file #1560

Merged
merged 102 commits into from May 15, 2019

Conversation

Dominik-Vogel
Copy link
Contributor

@Dominik-Vogel Dominik-Vogel commented May 13, 2019

Configurable Station

This PR integrates the StationConfigurator from qdev-dk/qdev-wrappers#68 into the QCoDeS station object.

(see #1256 for complete discussion history)

The StationConfigurator has been a separate object in qdev_wrappers because the Station lives in qcodes core, but it conceptually belongs into the Station.
The Station is meant to be a representation of the physical setup. And this description is given by the station config yaml file.

What the Station does

So far the station has methods for snapshotting:

  • snapshot_base
  • add_component
    and default measurements:
  • set_measurement
  • measure

Additions

and in this pr methods for serialization from a yaml are added:

  • load_config_file
  • load_instrument

I suggest that we deprecate the default measurement functionality in another PR, because as far as I know it is not used and it is only used by the legacy loop. Removing these functions would further decouple dependencies.

Migrating from the StationConfigurator

Previous bootstrapping looked like this:

  • create Station (no arguments)
  • create StationConfigurator, pass station
    (- call station specific init method/script to specify sample_name see point 2 in future improvements.)

With this an experiment is bootstrapped by simply:
-create station (potentially with sample name as argument)

Moving from the StationConfigurator to the new Station object will require two simple changes:

  • change <station_configurator_object>.load_instrument(...) to <station_obect>.load_instrument(...) and remove instantiation of <station_configurator_object>
  • change the configuration in ~\qcodesrc.json by replacing the section name station_configurator by station

Future improvements

To go in small steps this PR addresses the minimal integration and documentation of the station configurator in the station. In later PRs documentation and possible refactorings of the internal structure might follow. Lets think about what we are committing to with this PR.

The next steps are:

  1. add support for virtual instruments:
    - add source_instrument as an attribute of the add_parameter section to allow to instantiation of DelegateParameters with a source from another instrument.
    - enable to automatically load those instruments if they are defined in the same yaml file.
    - add a suitable base class for a purely virtual instrument.
  2. Accommodate the control over the sample name in the Station object. (this would also satisfy @ThorvaldLarsen s request.)

Dominik-Vogel and others added 30 commits September 3, 2018 15:42
astafan8 added 13 commits May 9, 2019 15:29
Mikhail Astafev.

When building docs locally, the “dataset context manager”
example notebook takes too much time to execute to_xarray
in the scatter plot example.

is it because xarray is trying to make a grid out of 5000 ungridded points?

It's weird because the conversion is executed on a 10-rows
dataframe - a very small one. But the resulting xarray
has 5000 in its dimensions, as if it was made out of the
original huge dataframe.

I tried with "copy" - still the same problem. to_dict, to_json,
etc - work on a copy, no problem, but to_xarray goes nuts and
processes all the 5000 rows from the parent dataframe which it
should not know about since I made a “copy”.
of Station example notebook
not the old 'station_configurator'
... in Dataset context manager example notebook.

Based on this pandas-dev/pandas#3686.
@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #1560 into master will increase coverage by 0.48%.
The diff coverage is 93.22%.

@@            Coverage Diff             @@
##           master    #1560      +/-   ##
==========================================
+ Coverage   71.54%   72.03%   +0.48%     
==========================================
  Files         105      105              
  Lines       12139    12304     +165     
==========================================
+ Hits         8685     8863     +178     
+ Misses       3454     3441      -13

@astafan8
Copy link
Contributor

@jenshnielsen @WilliamHPNielsen please, review this. I did my review in the previous PR, we need fresh eyes :)

Copy link
Collaborator

@jenshnielsen jenshnielsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I left some minor comments inline

docs/community/objects.rst Outdated Show resolved Hide resolved
environment.yml Show resolved Hide resolved
qcodes/config/qcodesrc_schema.json Outdated Show resolved Hide resolved
qcodes/instrument/parameter.py Show resolved Hide resolved
qcodes/instrument/parameter.py Outdated Show resolved Hide resolved
qcodes/station.py Show resolved Hide resolved
qcodes/station.py Show resolved Hide resolved
qcodes/station.py Outdated Show resolved Hide resolved
qcodes/station.py Outdated Show resolved Hide resolved
qcodes/utils/helpers.py Show resolved Hide resolved
@Dominik-Vogel
Copy link
Contributor Author

Thanks Jens for all the good comments, I'll work on them.

qcodes/station.py Outdated Show resolved Hide resolved
@WilliamHPNielsen
Copy link
Contributor

WilliamHPNielsen commented May 15, 2019

I have no objections.

-Haha, it's really cool work! Good job.

@jenshnielsen jenshnielsen merged commit 9bf6357 into microsoft:master May 15, 2019
giulioungaretti pushed a commit that referenced this pull request May 15, 2019
Merge: f38ff8b 2e18e2f
Author: Jens Hedegaard Nielsen <Jens.Nielsen@microsoft.com>

    Merge pull request #1560 from Dominik-Vogel/station_configurator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants