Adding a logger widget for logging in run_tardis #2700

DeekshaMohanty · 2024-07-12T15:51:27Z

📝 Description

Fixes #2701

Type: 🚀 feature

Added an output ipyidget with tabs to logger.py
Removed colored_logger.py

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.

tardis-bot · 2024-07-12T15:52:27Z

*beep* *bop*

Hi, human.

The docs workflow has failed ❌

Click here to see the build log.

review-notebook-app · 2024-07-12T18:00:02Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

tardis-bot · 2024-07-12T18:01:01Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

…NG/ERROR" instead of the initial five tabs

tardis-bot · 2024-07-16T15:22:13Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

tardis-bot · 2024-07-16T18:13:39Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

… INFO, DEBUG and ALL.

tardis-bot · 2024-07-18T17:57:48Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

…8, where log level "ALL" was not added.

tardis-bot · 2024-07-22T22:00:24Z

*beep* *bop*

Hi, human.

I'm the @tardis-bot and couldn't find your records in my database. I think we don't know each other, or you changed your credentials recently.

Please add your name and email to .mailmap in your current branch and push the changes to this pull request.

In case you need to map an existing alias, follow this example.

wkerzendorf · 2024-07-24T18:37:50Z

@DeekshaMohanty can you please fix the .mailmap problem.

…ls in one place.

tardis-bot · 2024-07-28T22:50:29Z

*beep* *bop*
Hi human,
I ran ruff on the latest commit (ef2b439).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Complete output(might be large):

DeekshaMohanty · 2024-07-28T23:09:33Z

Points 6 and 7 for the issue linked to this PR need some further discussion. #2701

codecov · 2024-07-29T15:40:44Z

Codecov Report

Attention: Patch coverage is 89.04110% with 8 lines in your changes missing coverage. Please review.

Project coverage is 69.32%. Comparing base (f8f664c) to head (ef2b439).
Report is 38 commits behind head on master.

Files	Patch %	Lines
tardis/io/logger/logger.py	89.04%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2700      +/-   ##
==========================================
- Coverage   69.68%   69.32%   -0.36%     
==========================================
  Files         181      190       +9     
  Lines       14469    15016     +547     
==========================================
+ Hits        10082    10410     +328     
- Misses       4387     4606     +219

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jaladh-singhal

This is a great addition @DeekshaMohanty - thanks for taking initiative on this!

I've left some comments, please address them when you get a chance. We need to make sure that we address all the points in #2701

jaladh-singhal · 2024-07-31T02:07:29Z

docs/quickstart.ipynb

@@ -116,7 +116,7 @@
    "sim = run_tardis(\"tardis_example.yml\", \n",
    "                 virtual_packet_logging=True,\n",
    "                 show_convergence_plots=True,\n",
-    "                 export_convergence_plots=True,\n",
+    "                 export_convergence_plots=False, #TODO: to avoid double plots\n",


Maybe we need to replace this parameter with a new parameter (maybe called export_static_output) which can show convergence plots and logger-widget (or atleast a fixed height html text area) for static HTML documentation website.

And setting this param to True should be controllable by identifying if the notebook execution is being done by nbsphinx/nbconvert - there should be some setting to detect that but needs research.

jaladh-singhal · 2024-07-31T02:09:25Z

tardis/io/logger/logger.py

-
-        logging_level = (
-            log_level if log_level else tardis_config["debug"]["log_level"]
+            tardis_config["debug"].get("specific_log_level", specific_log_level)


good cleanup!

jaladh-singhal · 2024-07-31T02:13:51Z

tardis/io/logger/logger.py

+    "WARNING/ERROR": Output(layout=Layout(height='300px', overflow_y='auto')),
+    "INFO": Output(layout=Layout(height='300px', overflow_y='auto')),
+    "DEBUG": Output(layout=Layout(height='300px', overflow_y='auto')),
+    "ALL": Output(layout=Layout(height='300px', overflow_y='auto'))


you can define a function that returns a Output widget and call for each of the keys, instead of repeating code.

jaladh-singhal · 2024-07-31T02:15:36Z

tardis/io/logger/logger.py

+tab.set_title(2, "DEBUG")
+tab.set_title(3, "ALL")
+
+display(tab)


do we really need to do it global level? shouldn't display be inside some function/class that gets called/instantiated?

If not, maybe move it together with other global level code at L184

jaladh-singhal · 2024-07-31T02:17:22Z

tardis/io/logger/logger.py

+        elif record.levelno == logging.DEBUG:
+            color = 'blue'
+        else:
+            color = 'black'


it will be cleaner if colors come from a dictionary, instead of defining here

jaladh-singhal · 2024-07-31T02:21:06Z

tardis/simulation/base.py

@@ -517,7 +517,7 @@ def log_plasma_state(
        plasma_state_log["next_w"] = next_dilution_factor
        plasma_state_log.columns.name = "Shell No."

-        if is_notebook():
+        if False and is_notebook(): #TODO: remove it with something better


I think we can simply remove the if block since we are controlling display now

Added Output widgets for logging within Tabs

52bdf15

jaladh-singhal requested review from wkerzendorf and DeerWhale July 12, 2024 15:53

DeekshaMohanty marked this pull request as draft July 12, 2024 15:54

jaladh-singhal requested review from jaladh-singhal and jamesgillanders July 12, 2024 15:54

Fixed the text not showing in the logging tabs

3426a59

Changed the logger widget to display two tabs "DEBUG/INFO" and "WARNI…

998e6e3

…NG/ERROR" instead of the initial five tabs

Modified the tabs to have "WARNING/ERROR", "INFO" "DEBUG" in that order

208d02c

Made changes to the logger to have it show the 4 tabs- WARNING/ERROR,…

c82467a

… INFO, DEBUG and ALL.

Added documentation and cleaned up the code, fixed an issue in line 5…

e0365ac

…8, where log level "ALL" was not added.

DeekshaMohanty added 2 commits July 24, 2024 15:01

Added my email to the .mailmap file

28262b5

Re-added the 'LOGGING_LEVELS' dictionary to have all the logging leve…

ef2b439

…ls in one place.

DeekshaMohanty marked this pull request as ready for review July 28, 2024 23:04

DeekshaMohanty marked this pull request as draft July 28, 2024 23:06

jaladh-singhal requested changes Jul 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a logger widget for logging in run_tardis #2700

Adding a logger widget for logging in run_tardis #2700

DeekshaMohanty commented Jul 12, 2024 •

edited

Loading

tardis-bot commented Jul 12, 2024 •

edited

Loading

review-notebook-app bot commented Jul 12, 2024

tardis-bot commented Jul 12, 2024

tardis-bot commented Jul 16, 2024

tardis-bot commented Jul 16, 2024

tardis-bot commented Jul 18, 2024

tardis-bot commented Jul 22, 2024

wkerzendorf commented Jul 24, 2024

tardis-bot commented Jul 28, 2024

DeekshaMohanty commented Jul 28, 2024 •

edited

Loading

codecov bot commented Jul 29, 2024

jaladh-singhal left a comment

jaladh-singhal Jul 31, 2024

jaladh-singhal Jul 31, 2024

jaladh-singhal Jul 31, 2024

jaladh-singhal Jul 31, 2024

jaladh-singhal Jul 31, 2024

jaladh-singhal Jul 31, 2024

Adding a logger widget for logging in run_tardis #2700

Are you sure you want to change the base?

Adding a logger widget for logging in run_tardis #2700

Conversation

DeekshaMohanty commented Jul 12, 2024 • edited Loading

📝 Description

🚦 Testing

☑️ Checklist

tardis-bot commented Jul 12, 2024 • edited Loading

review-notebook-app bot commented Jul 12, 2024

tardis-bot commented Jul 12, 2024

tardis-bot commented Jul 16, 2024

tardis-bot commented Jul 16, 2024

tardis-bot commented Jul 18, 2024

tardis-bot commented Jul 22, 2024

wkerzendorf commented Jul 24, 2024

tardis-bot commented Jul 28, 2024

DeekshaMohanty commented Jul 28, 2024 • edited Loading

codecov bot commented Jul 29, 2024

Codecov Report

jaladh-singhal left a comment

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

jaladh-singhal Jul 31, 2024

Choose a reason for hiding this comment

DeekshaMohanty commented Jul 12, 2024 •

edited

Loading

tardis-bot commented Jul 12, 2024 •

edited

Loading

DeekshaMohanty commented Jul 28, 2024 •

edited

Loading