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.

Sign up for GitHub

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

Jump to bottom

[WIP] Add ability to configure logging for debugging #360

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+293 −256
Open

[WIP] Add ability to configure logging for debugging #360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
70e1cc1
Small cleaning up of the configuration module, to remove hardcoded AP…
josh146 Apr 11, 2020
2b1586b
fix docs building
josh146 Apr 11, 2020
11fdb1c
added logger
josh146 Apr 12, 2020
bee52ad
logging
josh146 Apr 12, 2020
86a35c5
blacking
josh146 Apr 12, 2020
98c7a73
add logging function
josh146 Apr 12, 2020
84bc9d9
Apply suggestions from code review
josh146 Apr 14, 2020
8390625
Merge branch 'master' into debug-logging
josh146 Apr 16, 2020
9f36842
Merge branch 'master' into debug-logging
josh146 Jun 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/code/sf_cli.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,4 @@ Code details
.. automodapi:: strawberryfields.cli
:no-heading:
:no-inheritance-diagram:
:skip: store_account, create_config, load, RemoteEngine, Connection, ConfigurationError, ping
:skip: store_account, DEFAULT_CONFIG, load, RemoteEngine, Connection, ConfigurationError, ping
31 changes: 29 additions & 2 deletions doc/code/sf_configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ and has the following format:
use_ssl = true
port = 443

[logging]
# Options for the logger
level = "warning"
logfile = "sf.log"

Configuration options
---------------------

Expand All @@ -41,7 +46,6 @@ Configuration options

Settings for the Xanadu cloud platform.


**authentication_token (str)** (*required*)
API token for authentication to the Xanadu cloud platform. This is required
for submitting remote jobs using :class:`~.RemoteEngine`.
Expand All @@ -63,6 +67,29 @@ Settings for the Xanadu cloud platform.

*Corresponding environment variable:* ``SF_API_PORT``

``[logging]``
^^^^^^^^^^^^^

Settings for the Strawberry Fields logger.

**level (str)** (*optional*)
Specifies the level of information that should be printed to the standard
output. Defaults to ``"info"``, which indicates that all logged details
are displayed as output.

Other options include ``"error"``, ``"warning"``, ``"info"``, ``"debug"``,
in decreasing levels of verbosity.

*Corresponding environment variable:* ``SF_LOGGING_LEVEL``

**logfile (str)** (*optional*)
The filepath of an output logfile. This may be a relative or an
absolute path. If specified, all logging data is appended to this
file during Strawberry Fields execution.

*Corresponding environment variable:* ``SF_LOGGING_LOGFILE``


Functions
---------

Expand All @@ -73,5 +100,5 @@ Functions

.. automodapi:: strawberryfields.configuration
:no-heading:
:skip: user_config_dir, store_account, active_configs, reset_config, create_logger
:skip: user_config_dir, store_account, active_configs, reset_config, create_logger, delete_config
:no-inheritance-diagram:
2 changes: 2 additions & 0 deletions strawberryfields/api/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ def create_job(self, target: str, program: Program, run_options: dict = None) ->

circuit = bb.serialize()

self.log.debug("Submitting job\n%s", circuit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to keep the level consistent here and on line 164: self.log.info("Job %s was successfully submitted.", job_id). Either both could be DEBUG or both could be INFO.

One thing I was wondering about: for errors, how should we handle logging? Would it make sense to have the same message when instantiating the error and also log that at the same time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! Errors should definitely be logged, the generated log files should have a running commentary of what happens during program execution.

One way of doing this could be using logger.exception.

This article seems to provide a nice overview of potential logging/exception patterns: https://www.loggly.com/blog/exceptional-logging-of-exceptions-in-python/

Copy link
Member Author

Choose a reason for hiding this comment

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

I originally had this as info, but I'm not so sure they both belong at the same level. From the Python docs:

Level Description
DEBUG Designates fine-grained informational events that are most useful to debug an application.
INFO Designates informational messages that highlight the progress of the application at coarse-grained level.

  • Job submission provides information about the progress of the job submission at a very course-grained level. Seems particularly well suited, given that info is the default level, and will be displayed on the output by default.

  • Logging the submitted circuit is instead more fine-grained debugging data. In most cases, users won't want this displayed as 'info', since the submitted circuit should be evident from their Python code (or determined via prog.compile()). However, this will be useful information when debugging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good! Happy if these are kept as they are right now :)


path = "/jobs"
response = requests.post(self._url(path), headers=self._headers, json={"circuit": circuit})
if response.status_code == 201:
Expand Down
1 change: 1 addition & 0 deletions strawberryfields/api/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def refresh(self):
self.log.debug("Job %s metadata: %s", self.id, job_info.meta)
if self._status == JobStatus.COMPLETED:
self._result = self._connection.get_job_result(self.id)
self.log.info("Job %s is complete", self.id)

def cancel(self):
"""Cancels an open or queued job.
Expand Down
4 changes: 2 additions & 2 deletions strawberryfields/cli/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import sys

from strawberryfields.api import Connection
from strawberryfields.configuration import ConfigurationError, create_config, store_account
from strawberryfields.configuration import ConfigurationError, DEFAULT_CONFIG, store_account
from strawberryfields.engine import RemoteEngine
from strawberryfields.io import load

Expand Down Expand Up @@ -162,7 +162,7 @@ def configuration_wizard():
Returns:
dict[str, Union[str, bool, int]]: the configuration options
"""
default_config = create_config()["api"]
default_config = DEFAULT_CONFIG["api"]

# Getting default values that can be used for as messages when getting inputs
hostname_default = default_config["hostname"]
Expand Down
Loading