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

Feature : an elapsed time parameter #1629

Merged
merged 11 commits into from
Jul 17, 2019

Conversation

WilliamHPNielsen
Copy link
Contributor

There has been recurring requests to get a default QCoDeS parameter to measure elapsed wall clock time in an experiment. This PR introduces a simple ElapsedTimeParameter to conveniently measure - you guessed it - elapsed wall clock time.

Changes proposed in this pull request:

  • Add a parameter to do the job
  • Add an example notebook

@QCoDeS/core

@WilliamHPNielsen
Copy link
Contributor Author

I realise that some users might like to have a parameter to measure more absolute time (time stamps). I think that is beyond the scope of this PR.

@codecov
Copy link

codecov bot commented Jul 12, 2019

Codecov Report

Merging #1629 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1629      +/-   ##
==========================================
+ Coverage   66.55%   66.58%   +0.03%     
==========================================
  Files         140      141       +1     
  Lines       17389    17406      +17     
==========================================
+ Hits        11573    11590      +17     
  Misses       5816     5816

Copy link
Contributor

@astafan8 astafan8 left a comment

Choose a reason for hiding this comment

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

Brilliant :) (almost, see comments :) )

qcodes/utils/time.py Outdated Show resolved Hide resolved
qcodes/utils/time.py Outdated Show resolved Hide resolved
qcodes/utils/time.py Outdated Show resolved Hide resolved
"from qcodes.utils.time import ElapsedTimeParameter\n",
"from qcodes.instrument.parameter import Parameter\n",
"from qcodes.dataset.measurements import Measurement\n",
"from qcodes.dataset.sqlite.database import initialise_or_create_database_at\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not yet comfortable with exposing where this function comes from. i prefer that for now it comes just from qcodes., otherwise people will start looking into what else they can use from .sqlite.* while they are not allowed because it's all our internal stuff that can change, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved one import to resolve this. We should make a separate PR that attacks the mess of the qcodes namespace.

"source": [
"# Measuring X as a function of time\n",
"\n",
"Sometimes we'd like to measure something as a function of elapsed wall clock time. This notebook provides a simple example of doing that using the `ElapsedTimeParameter`."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend to add here the reason behind ElapsedTimeParameter - the fact that qcodes provides it, hence all qcodes users in principle can use the same parameter which makes it easier to relate measurements which use this parameter (sorry, on friday evening my language skills are gone, but i hope you got what i mean :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a little bit. I'm not too sure what you are after... -do we need to document why standardization is helpful?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, in a subtle way, like "this parameter provides a single way of measuring time which is awesome because standardization is awesome in this case" :)

@WilliamHPNielsen
Copy link
Contributor Author

After discussions with Jens, I have moved the code from utils to a new module next to the parameters. I agree that this makes more sense.

@jenshnielsen
Copy link
Collaborator

I think we should include this in the api docs in docs/api/parameter/* Just let me know if you need a hand setting that up correctly

@WilliamHPNielsen
Copy link
Contributor Author

@jenshnielsen was this what you had in mind?

@jenshnielsen jenshnielsen merged commit 7a32e9f into microsoft:master Jul 17, 2019
giulioungaretti pushed a commit that referenced this pull request Jul 17, 2019
Merge: e7b46a0 2801cec
Author: Jens Hedegaard Nielsen <Jens.Nielsen@microsoft.com>

    Merge pull request #1629 from WilliamHPNielsen/feature/time_parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants