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

Better logging #88

Closed
atilag opened this issue Sep 28, 2017 · 7 comments
Closed

Better logging #88

atilag opened this issue Sep 28, 2017 · 7 comments
Labels
good first issue Good for newcomers type: enhancement It's working, but needs polishing

Comments

@atilag
Copy link
Member

atilag commented Sep 28, 2017

Welcome contributors!

So far, we've been using a flag called "silent" (you can search for it into the sources), that if set to true, will cause some information to be printed out to the standard output.
We want to change it!
So if you are willing to contribute to this amazing project (I know you are!), I'd suggest you to start with something simple like this issue.
We want to change all the "silent" flag behavior with a more robust and standard logging solution based in the python logging facility. So basically these are the tasks we need to address:

  • Incorporate standard python logging module

  • Create a logging config file with decent default settings:

    • INFO level
    • Console output
    • Set "disable_exisiting_loggers: false"
    • format: "%(asctime)s : %(name)s : %(levelname)s : %(message)s"
  • Loggers should be at module level and use the __name__ attribute:

    • logger = logging.getLogger(__name__)
  • Remove "silent" flag everywhere and replace the print() statements with logger.debug() (we don't want to show messages by default)

Suggestions are welcome! :)

Notes

  • If you want to take this bug, let us know and we will assign you to it.
  • You can open a PR to "master" branch with the word: "WIP" (Work In Progress) in front of the description, so we will know that you haven't finished and we can make parcial reviewings. We will eliminate the WIP prefix once the PR is ready to be merged.
  • Make sure all tests pass! :)

Current Behavior

We are using the "silent" variable to print out information via standard output in just some modules. We want to remove them all and use a common logging system.

@atilag atilag added type: enhancement It's working, but needs polishing good first issue Good for newcomers labels Sep 28, 2017
ewinston pushed a commit that referenced this issue Oct 11, 2017
* Add logging utils and "QuantumProgram.set_verbose"

Add the basis for being able to replace the existing "print" and
"verbose"/"silent" flags with standard logging messages:
* add a `qiskit._logging` module that contains two convenience functions
  for setting up the loggers (format, handlers) to be used by the SDK,
  defaulting to sensible values.
* add `QuantumProgram.enable_logs()` and `disable_logs()` as the single
  point of entry for setting/unsetting the verbosity of the output.

* Remove silent and verbose arguments, adjust tests

Remove the `silent` and `verbose` arguments on the codebase, and change
the `print` statements into `logger.info` statements in the main modules
and into `logger.debug` at `qiskit.mapper` due to the verbosity of the
output. Adjust the tests accordingly.

Note that the `qiskit.tools` module has not been updated, and that in
some cases `print(a,b)` had to be converted into
`logger.info("%s%s" % (a, b))` as the calls expect the full string to
be passed as the first parameter.

* Add note about logging on sphinx documentation

* Revise log calls to pass the args directly

Modify `log.info('some %s' % thing)` to `log.info('some %s', thing)` in
order to avoid introducing linting warnings.

* Fix typo on documentation for logging

* Simple format for INFO, revert get_execution_list

Use a custom formatter "SimpleInfoFormatter" on the logging
configuration enabled by "QuantumProgram.enable_logs()" that makes the
INFO messages displayed in a simple format (no timestamp or module
information). The rest of the levels retain the previous formatting.

Revert QuantumProgram.get_execution_list() to using print statements,
on the principle that it will be only be used during interactive
sessions.
@hellmersjl3ds
Copy link
Contributor

What's the status of this? I see a commit from @ewinston but it's not assigned to anyone.

I'd be happy to work on it if nobody else is.

@ewinston
Copy link
Contributor

Hi @hellmersjl3ds,
Thanks for offering to have a look at this. In PR#108, referenced in this PR, the python logging facility was setup and covers most of the features requested in this enhancement. One thing I think that remains to be done, however, is that the facility has not been applied everywhere (there are still print statements for instance). If you could help make the logging uniform or suggest any other improvements that would be great. For extensive changes you could open a new issue for discussion or continue using this one.

@1ucian0
Copy link
Member

1ucian0 commented Jan 8, 2018

Not every print call in the code should be replaced by log. Some of them need a thought. Following, a list of files in 0.4, with prints in it, and some comments for some of the cases:

  • qiskit/tools/visualization.py
  • qiskit/tools/qi/qi.py
  • qiskit/tools/qi/pauli.py
  • qiskit/tools/apps/optimization.py
  • qiskit/unroll/_printerbackend.py (probably makes sense to add a file parameter, a la build-in print )
  • qiskit/qasm/_qasmlexer.py: the print call in t_error maybe is an Exception (?)
  • qiskit/qasm/_node/*.py: Many prints happen in to_string methods. This method should return a string instead of printing (maybe should be called __str__)
  • qiskit//qasm/_qasmparser.py: The exception should be re-raised.

Currently, the testsuite do not print any of these issues (I think, it could be that the output is captured in some cases). For sure, tests are needed when performing many of these changes.

This is a very nice opportunity for a first contribution. If anybody wants to work with it and need assistance, let me know! (<- @hellmersjl3ds)

@atilag
Copy link
Member Author

atilag commented Jan 8, 2018

Not every print call in the code should be replaced by log.

The problem of using print()statements, is that it forces the user to dump something to the stdout (usually the monitor), but this is not true for many other uses cases that printing to stdout doesn't make any sense like:

  • Graphics User Interfaces (Windows apps, Mac apps, XWindows apps)
  • Non-Attended devices
  • *nix Daemons and Services
  • ... and probably many others I'm not aware of...

We are building an SDK, so using print() is killing many use-cases we don't really want to kill.
So, basically, yes, we should replace al the prints in the code by log :)
Notice that our logger can dump to stdout like print always does, if the user requires to do so... not sure if we are exposing this functionality to the user at the moment, though.

@1ucian0
Copy link
Member

1ucian0 commented Jan 12, 2018

Some of the prints I mentioned are messages for an exception, for example.

@hellmersjl3ds
Copy link
Contributor

Hey guys, unfortunately I'm busy with another project for the next 5 weeks or so, so I won't be able to get to this before then.

@atilag
Copy link
Member Author

atilag commented Feb 8, 2018

Ok, so after some PRs... we have included the logging system and we have removed all silent flags.
Still need to remove all presence of print() in the code, but we will create another issue to address this specific issue.
I'm going to close this Issue.

@atilag atilag closed this as completed Feb 8, 2018
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
…it#97 (Qiskit#102)

* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
…kit#114)

* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95

* added Contributin to Qiskit

fixes Qiskit#112
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95

* added Contributin to Qiskit

fixes Qiskit#112

* remove Aer install instructions

fixes Qiskit#84

* Have the ToC do the work, instead of a landing page for Getting Started.

fixes Qiskit#84
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95

* added Contributin to Qiskit

fixes Qiskit#112

* remove Aer install instructions

fixes Qiskit#84

* Have the ToC do the work, instead of a landing page for Getting Started.

fixes Qiskit#84

* circuit drawing style in Getting Started

begins to address Qiskit#98

* adopting the term of art 'quantum cloud services'
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95

* added Contributin to Qiskit

fixes Qiskit#112

* remove Aer install instructions

fixes Qiskit#84

* Have the ToC do the work, instead of a landing page for Getting Started.

fixes Qiskit#84

* circuit drawing style in Getting Started

begins to address Qiskit#98

* adopting the term of art 'quantum cloud services'

* getting_started references wrong circuit drawing PNG file

Fixes Qiskit#129
taalexander pushed a commit to taalexander/qiskit-terra that referenced this issue May 2, 2019
* moved Aer tutorial images to /docs/images/figures/

and all references to these images

* Have Makefile point to correct Aqua and Chemistry directories

Fixes Qiskit#88

* Revert "Have Makefile point to correct Aqua and Chemistry directories"

This reverts commit fee13ba87e1888c88a997430ee0a57969a9985ac.

* Create community_qiskit.rst

* Revert "Create community_qiskit.rst"

This reverts commit 5fd2cafd6d2e570488e9985f26d33a0f5088c581.

* update global install instructions

Motivation: to synthesize the mostly duplicate install instructions currently under each Qiskit element's doc section, which can then be removed.

Direct to CONTRIBUTING files in each element's repo for instructions to build from source, since this serves a niche persona and is lengthy.

Added images for steps that involve pointing and clicking through the web.

Rewrote material for additional clarity.

* removed Troubleshooting section

fixes Qiskit#97

* simplify the ibm q provider documentation

Fixes Qiskit#86

* removed Visualizations from global ToC, honed Installing QIskit

Fixes Qiskit#107 and fixes Qiskit#95

* added Contributin to Qiskit

fixes Qiskit#112

* remove Aer install instructions

fixes Qiskit#84

* Have the ToC do the work, instead of a landing page for Getting Started.

fixes Qiskit#84

* circuit drawing style in Getting Started

begins to address Qiskit#98

* adopting the term of art 'quantum cloud services'

* getting_started references wrong circuit drawing PNG file

Fixes Qiskit#129

* Add instructions on how to create a provider

fixes  Qiskit#123

* fix a typo in contributing_to_qiskit.rst

* update instructions on how to create a provider

addressing review by @yaelbh

* ipython3 to python code blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: enhancement It's working, but needs polishing
Projects
None yet
Development

No branches or pull requests

4 participants