Implementing Logging Configuration for Notebook Logging #1633

DhruvSondhi · 2021-06-07T18:28:08Z

This PR aims to implement a configuration for the Notebook logging such that logging can be turned on, off or set to a particular log level for better information.

Description

This implementation of the logging function allows for the following possible state:

Log_level, a string value which can be any of the following values: [Notset, Debug, Info, Warning, Error, Critical], allows for logging the output at any of these mention levels
Default value has been set to "Critical", thus no logging output is generated.

Implementation via YAML has also been done. New log_state flag is used to control the logging level as well as the state of the logger.
A new Debug section has been created for the JSON Schema. This section caters to all the Debugging parameters that can be set for TARDIS configuration. Debug Packets will also be moved here soon after discussion.

Motivation and context

Allowing for configurable logging levels & state would be a good addition to TARDIS. It would allow for running the simulation without the logs for graphs & other relevant information. Also, gives the opportunity to get only the specified information from the logger for the simulation ie logs of a particular level.

Screenshots

The screenshots here shows some of the different settings & their corresponding output for the logging configuration.

Without specifying any flag:

The logger doesn't print any information for the Simulation.

Specifying the log_state flag (functional argument) {setting to "Debug" Level}:

The simulation runs for 2 iterations.

Specifying the logging_level: flag (YAML argument) {setting to "Warning" Level}:

The simulation runs for 2 iterations.

Specifying the logging_level: as well as log_state flag (functional & YAML argument, both specified) {setting to "Info" Level for functional parameter & "Warning" Level for YAML}:

The simulation runs for 2 iterations.
Note: Log_state & log_state {YAML file parameter} both specified & Log_state will be used for Log Level Determination will be printed for the user to notify that both parameters have been given. Later on this will implemented via a logging statement as well.

Documentation Preview : https://dhruvsondhi.github.io/tardis/branch/notebook_logging_config/io/optional/logging_configuration.html

How has this been tested?

Testing pipeline.
Other.

Type of change

Bug fix.
New feature.
Breaking change.
None of the above.

Checklist

My change requires a change to the documentation.
- I have updated the documentation accordingly.
- (optional) I have built the documentation on my fork following the instructions.
I have assigned and requested two reviewers for this pull request.

tardis-bot · 2021-06-07T18:28:28Z

Before a pull request is accepted, it must meet the following criteria:

Is the necessary information provided?
Is this a duplicate PR?
- If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
- If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
Does it pass existing tests and are new tests provided if required?
- The test coverage should not decrease, and for new features should be close to 100%.
Is the code tidy?
- No unnecessary print lines or code comments.

codecov · 2021-06-07T18:38:28Z

Codecov Report

Merging #1633 (eb3cb7a) into master (c47a3df) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head eb3cb7a differs from pull request most recent head 37388fc. Consider uploading reports for the commit 37388fc to get more accurate results

@@            Coverage Diff             @@
##           master    #1633      +/-   ##
==========================================
- Coverage   61.89%   61.86%   -0.03%     
==========================================
  Files          62       62              
  Lines        5732     5738       +6     
==========================================
+ Hits         3548     3550       +2     
- Misses       2184     2188       +4

Impacted Files	Coverage Δ
tardis/tardis/simulation/base.py	`83.33% <0.00%> (-1.84%)`	⬇️
tardis/tardis/base.py	`59.09% <0.00%> (+4.09%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c47a3df...37388fc. Read the comment docs.

tardis/base.py

tardis/io/schemas/base.yml

tardis/io/schemas/debug.yml

tardis/montecarlo/montecarlo_numba/montecarlo_logger.py

wkerzendorf · 2021-06-09T21:17:51Z

Please add documentation to this.

DhruvSondhi · 2021-06-10T07:00:35Z

Also, This needs to be moved to a better place in the CodeBase. Need to discuss where a new Logging Submodule can be created 🚀

tardis/__init__.py

tardis/io/schemas/debug.yml

tardis/__init__.py

tardis/simulation/tests/test_simulation.py

Changed the TestSimulationLogging class to comply to PEP8 styling

… & specific resp

Rodot-

I would make sure input to this function is validated, or at least don't let it pass silently without warning or error if an invalid value is encountered

tardis/__init__.py

Changed some implementation based on requested changes

…is passed

tardis/__init__.py

… arguments Default value now falls to Debug->Log_state variable Log_state functional parameter has None as default value

…n YAML & Functional parameters are specified Updated the Documentation for the newer changes

I approved the changes

* Implementing Logging Configuration for Notebook Logging via Function * Adding Docstring to logging_state() * Making logging states independent of case of arugment passed * Adding Debug YAML for Logging & Debugging * Changing implementation to a single unified flag for logging level * Moved logging_state() from montecarlo_logger to __init__ for TARDIS * Adding support for logging messages at specified level * Moving check for yaml & functional argument to logging_state function * Added functionality for the Debug section of the YAML to be optional Implemented check for Logging level & displaying Dataframe appropriately * Added Tests for logging output configuration via Functional Input & YAML File Added new YAML test file for testing YAML arguments for logging configuation * Fixed display() still showing the plasma stratification values table when using specific logging at other logging levels than INFO * Reworded description of the specific_logging parameter in debug.yml * Added tests for simulation when both parameters, functional & YAML, are present Changed debug.yml to be consistent with other test YAML configurations * Changed the number of iterations & test cases for faster test executions * Fixed docstring in __init__.py to impart the current implementation of the logging_state func * Restructuring tests for faster simulation runs * Added Tutorial Documentation for logging configuration * Fixed & reworded some explaination in the Tutorial for better understanding * Changed Quickstart notebook to incorporate new logging config in run_tardis() * Changed the notebook as request, grammer changes * Changed the python version to 3.7.10, reran the notebooks * Fixed some inconsistencies in tutorial grammar * Fixed with requested changes * Made logging_level constant value Changed the TestSimulationLogging class to comply to PEP8 styling * Fixed metadata for scroll in quickstart notebook * Renamed logging_level & specific_logging in debug schema to log_state & specific resp * Added explaination of list_of_filters Changed some implementation based on requested changes * Added raising of an expection when invalid log_state parameter value is passed * Changed implementation for the log_state function as well as the YAML arguments Default value now falls to Debug->Log_state variable Log_state functional parameter has None as default value * Added message for informing the user which parameter will be used when YAML & Functional parameters are specified Updated the Documentation for the newer changes * [build docs]

DhruvSondhi requested review from wkerzendorf, andrewfullard and Rodot- June 7, 2021 18:28

DhruvSondhi added feature request 🚀 feature work in progress 🚧 labels Jun 7, 2021

DhruvSondhi marked this pull request as draft June 8, 2021 04:02

DhruvSondhi force-pushed the notebook_logging_config branch from 3c55991 to 900307b Compare June 8, 2021 04:32

andrewfullard requested changes Jun 9, 2021

View reviewed changes

DhruvSondhi force-pushed the notebook_logging_config branch from 9d48f12 to fa0a59e Compare June 9, 2021 18:29

DhruvSondhi requested a review from andrewfullard June 9, 2021 19:51

DhruvSondhi force-pushed the notebook_logging_config branch from 8388c36 to 93d3cb4 Compare June 9, 2021 19:51

DhruvSondhi marked this pull request as ready for review June 10, 2021 05:28

DhruvSondhi force-pushed the notebook_logging_config branch 2 times, most recently from faa327d to 09cf417 Compare June 14, 2021 15:56

wkerzendorf reviewed Jun 15, 2021

View reviewed changes

tardis/__init__.py Show resolved Hide resolved

wkerzendorf reviewed Jun 15, 2021

View reviewed changes

tardis/__init__.py Show resolved Hide resolved

DhruvSondhi force-pushed the notebook_logging_config branch 2 times, most recently from 2c0d0a3 to e69d467 Compare June 17, 2021 16:02

KevinCawley mentioned this pull request Jun 17, 2021

Add SnakeViz profiling example #1657

Merged

10 tasks

DhruvSondhi force-pushed the notebook_logging_config branch 2 times, most recently from cafd781 to 1efe8cb Compare June 18, 2021 08:55

DhruvSondhi requested a review from wkerzendorf June 18, 2021 13:42

andrewfullard reviewed Jun 18, 2021

View reviewed changes

tardis/io/schemas/debug.yml Outdated Show resolved Hide resolved

wkerzendorf reviewed Jun 18, 2021

View reviewed changes

tardis/__init__.py Outdated Show resolved Hide resolved

wkerzendorf reviewed Jun 18, 2021

View reviewed changes

tardis/simulation/tests/test_simulation.py Show resolved Hide resolved

DhruvSondhi added 4 commits June 29, 2021 21:10

Fixed with requested changes

928000d

Made logging_level constant value

1d4cb5f

Changed the TestSimulationLogging class to comply to PEP8 styling

Fixed metadata for scroll in quickstart notebook

be1da5d

Renamed logging_level & specific_logging in debug schema to log_state…

d8ec785

… & specific resp

DhruvSondhi force-pushed the notebook_logging_config branch from 98cda65 to ba9f67e Compare June 29, 2021 15:41

Rodot- requested changes Jun 29, 2021

View reviewed changes

tardis/__init__.py Outdated Show resolved Hide resolved

Rodot- approved these changes Jun 29, 2021

View reviewed changes

Rodot- self-requested a review June 30, 2021 13:16

Rodot- requested changes Jun 30, 2021

View reviewed changes

DhruvSondhi requested a review from Rodot- June 30, 2021 15:54

Rodot- requested changes Jun 30, 2021

View reviewed changes

tardis/__init__.py Outdated Show resolved Hide resolved

tardis/__init__.py Outdated Show resolved Hide resolved

DhruvSondhi added 2 commits June 30, 2021 23:34

Added explaination of list_of_filters

c8ea04e

Changed some implementation based on requested changes

Added raising of an expection when invalid log_state parameter value …

fd947a2

…is passed

DhruvSondhi force-pushed the notebook_logging_config branch from fbb4c3c to 3a1f29f Compare June 30, 2021 18:04

DhruvSondhi requested a review from Rodot- June 30, 2021 18:06

Rodot- reviewed Jun 30, 2021

View reviewed changes

tardis/__init__.py Outdated Show resolved Hide resolved

DhruvSondhi force-pushed the notebook_logging_config branch from 2e50fee to 6d21a61 Compare June 30, 2021 18:27

DhruvSondhi requested review from Rodot- and andrewfullard June 30, 2021 18:27

Changed implementation for the log_state function as well as the YAML…

f63b8d5

… arguments Default value now falls to Debug->Log_state variable Log_state functional parameter has None as default value

DhruvSondhi force-pushed the notebook_logging_config branch from 6d50c24 to f63b8d5 Compare June 30, 2021 19:03

Added message for informing the user which parameter will be used whe…

f23f59e

…n YAML & Functional parameters are specified Updated the Documentation for the newer changes

DhruvSondhi force-pushed the notebook_logging_config branch from 70109cf to f23f59e Compare July 1, 2021 12:49

[build docs]

37388fc

andrewfullard approved these changes Jul 1, 2021

View reviewed changes

Rodot- approved these changes Jul 1, 2021

View reviewed changes

andrewfullard merged commit 8b7908b into tardis-sn:master Jul 1, 2021

DhruvSondhi deleted the notebook_logging_config branch July 1, 2021 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing Logging Configuration for Notebook Logging #1633

Implementing Logging Configuration for Notebook Logging #1633

DhruvSondhi commented Jun 7, 2021 •

edited

Loading

tardis-bot commented Jun 7, 2021

codecov bot commented Jun 7, 2021 •

edited

Loading

wkerzendorf commented Jun 9, 2021 •

edited

Loading

DhruvSondhi commented Jun 10, 2021

Rodot- left a comment

Implementing Logging Configuration for Notebook Logging #1633

Implementing Logging Configuration for Notebook Logging #1633

Conversation

DhruvSondhi commented Jun 7, 2021 • edited Loading

tardis-bot commented Jun 7, 2021

codecov bot commented Jun 7, 2021 • edited Loading

Codecov Report

wkerzendorf commented Jun 9, 2021 • edited Loading

DhruvSondhi commented Jun 10, 2021

Rodot- left a comment

Choose a reason for hiding this comment

DhruvSondhi commented Jun 7, 2021 •

edited

Loading

codecov bot commented Jun 7, 2021 •

edited

Loading

wkerzendorf commented Jun 9, 2021 •

edited

Loading