NREL-Sienna / InfrastructureSystems.jl

Utility package for Sienna's simulation infrastructure
https://nrel-sienna.github.io/InfrastructureSystems.jl/
BSD 3-Clause "New" or "Revised" License
39 stars 20 forks source link

Add filtering of log messages by group #229

Closed daniel-thom closed 3 years ago

daniel-thom commented 3 years ago

This PR adds a simple solution to the problem where debug log messages from a subject like time series or serialization can overwhelm the log file and you can't debug what you want. Julia already had a solution; we just didn't realize it.

Refer to the changes I made to the time series source file as example usages. If you all like this then I will tag all of the debug messages in IS with appropriate labels.

codecov[bot] commented 3 years ago

Codecov Report

Merging #229 (05fa30f) into master (6b23389) will increase coverage by 0.09%. The diff coverage is 91.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
+ Coverage   75.14%   75.24%   +0.09%     
==========================================
  Files          43       43              
  Lines        3074     3114      +40     
==========================================
+ Hits         2310     2343      +33     
- Misses        764      771       +7     
Flag Coverage Δ
unittests 75.24% <91.04%> (+0.09%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/InfrastructureSystems.jl 60.00% <ø> (ø)
src/time_series_storage.jl 77.77% <0.00%> (ø)
src/component.jl 93.58% <75.00%> (ø)
src/serialization.jl 50.49% <80.00%> (ø)
src/hdf5_time_series_storage.jl 93.37% <90.00%> (ø)
src/utils/logging.jl 89.78% <91.76%> (-1.89%) :arrow_down:
src/components.jl 90.00% <100.00%> (ø)
src/deterministic_single_time_series.jl 75.86% <100.00%> (ø)
src/in_memory_time_series_storage.jl 71.84% <100.00%> (ø)
src/system_data.jl 86.26% <100.00%> (ø)
... and 6 more
daniel-thom commented 3 years ago

This is now more of a complete solution. It shows how tests can use the feature.

kdheepak commented 3 years ago

I didn't know about _group! Neat!

claytonpbarrows commented 3 years ago

One more nit: Is "min_level" the preferred terminology? What about "group_log_level" or "log_group_level"?

daniel-thom commented 3 years ago

One more nit: Is "min_level" the preferred terminology? What about "group_log_level" or "log_group_level"?

I started with set_min_level because group is a parameter name. set_min_group_level is probably better. The first argument has type MultiLogger, so I think log is not necessary.

daniel-thom commented 3 years ago

Here is a summary of changes I made based on today's discussion:

daniel-thom commented 3 years ago

Is the PSY test break expected?

No. I fixed the bug. Glad we had the cross-package tests.