tardis
tardis copied to clipboard
Adding Debug Messages to Log the Status of Simulation
This PR aims to add more debugging messages to the Simulation Logs to display the simulation's flow.
Description
Motivation and context
Screenshots
The new messages look something on these lines. Subject to iterative change.
How has this been tested?
- [x] Testing pipeline.
- [x] Other.
Type of change
- [ ] Bug fix.
- [x] New feature.
- [ ] Breaking change.
- [ ] None of the above.
Checklist
- [x] 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.
- [x] I have assigned and requested two reviewers for this pull request.
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.
There are some commented out sections of code in this PR. That will be taken care of once we have the discussion about what all needs to be debug logs.
Please keep in mind that there is a bug in the current implementation with the logging done inside the config_reader.py
. As the configuration
is created before calling the logging_state
function, the log_state
variable comes in effect from the next run of the simulation. This is because the logger for the config_reader
is changing the log state after the run of the simulation.
Codecov Report
Merging #1704 (80e8102) into master (f24be9a) will increase coverage by
1.31%
. The diff coverage isn/a
.
:exclamation: Current head 80e8102 differs from pull request most recent head b6dadc5. Consider uploading reports for the commit b6dadc5 to get more accurate results
@@ Coverage Diff @@
## master #1704 +/- ##
==========================================
+ Coverage 57.65% 58.97% +1.31%
==========================================
Files 66 66
Lines 6809 6864 +55
==========================================
+ Hits 3926 4048 +122
+ Misses 2883 2816 -67
Impacted Files | Coverage Δ | |
---|---|---|
.../montecarlo/montecarlo_numba/single_packet_loop.py | 27.11% <0.00%> (-2.67%) |
:arrow_down: |
tardis/tardis/simulation/base.py | 83.26% <0.00%> (-0.07%) |
:arrow_down: |
tardis/tardis/analysis.py | 30.16% <0.00%> (ø) |
|
tardis/tardis/visualization/tools/sdec_plot.py | 9.52% <0.00%> (ø) |
|
...rdis/tardis/montecarlo/montecarlo_configuration.py | 100.00% <0.00%> (ø) |
|
tardis/tardis/io/atom_data/base.py | 90.05% <0.00%> (+0.28%) |
:arrow_up: |
...s/tardis/visualization/widgets/custom_abundance.py | 14.42% <0.00%> (+0.39%) |
:arrow_up: |
tardis/tardis/model/density.py | 92.95% <0.00%> (+0.42%) |
:arrow_up: |
tardis/tardis/io/config_reader.py | 82.77% <0.00%> (+0.68%) |
:arrow_up: |
tardis/tardis/plasma/standard_plasmas.py | 73.83% <0.00%> (+0.71%) |
:arrow_up: |
... and 5 more |
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 f24be9a...b6dadc5. Read the comment docs.
As discussed - one PR - one idea . Can you please split up this PR in 3-4 PRs with a maximum of 2-3 files changed
As discussed - one PR - one idea . Can you please split up this PR in 3-4 PRs with a maximum of 2-3 files changed
Hello Wolfgang, this is not possible as this PR contains all the debug messages that are added wrt to the Flow chart that was created. Creating multiple PRs for the same defeats the purpose of the way the messages were added in order.
@DhruvSondhi What is going on with this PR? @andrewfullard had a couple of comments. I think we could merge after they are fixed.
Hello @wkerzendorf , I didn't have time to work on this PR due to the tracking functionality that was being developed. I am not completely sure on what all needs to be taken care here but I would do the housekeeping & get it ready for a review. Majority of the debug messages are now in their intended place.
Closing as stale- the work can be reused in the future.