adding RPacketPlotter to TARDIS

[jayantbhakar]

:pencil: Description

Type: :rocket: feature

This PR is based on the Montecarlo 2D packet visualization project. It aims to write a module for the RPacketPlotter tool which creates a 2D animated graph of the RPackets or the real packets used in the Montecarlo simulation.

Example code to plot is shown below:

rpacket_plotter = RPacketPlotter.from_simulation(sim, no_of_packets=10) rpacket_plotter.generate_plot()

Here's how the plot looks like:

Light Theme:

Dark theme:

Also, link issues affected by this pull request by using the keywords: close, closes, closed, fix, fixes, fixed, resolve, resolves or resolved.

:pushpin: Resources

Examples, notebooks, and links to useful references.

:vertical_traffic_light: Testing

How did you test these changes?

• [ ] Testing locally: Here is the log of pytests that I have run locally.

:ballot_box_with_check: 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.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 98.54369% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.67%. Comparing base (e88a31a) to head (d83ceae). Report is 1 commits behind head on master.

Files	Patch %	Lines
tardis/visualization/tools/rpacket_plot.py	96.73%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2119      +/-   ##
==========================================
+ Coverage   69.25%   69.67%   +0.42%     
==========================================
  Files         179      181       +2     
  Lines       14262    14468     +206     
==========================================
+ Hits         9877    10081     +204     
- Misses       4385     4387       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[jayantbhakar]

Yeah, I think before writing tests and docs, I should work on adding some functionalities that are left. Today I am working on the dark theme.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[tardis-bot]

*beep* *bop*

Hi, human.

The docs workflow has succeeded :heavy_check_mark:

Click here to see your results.

[sonachitchyan]

in here, change it from ```ith`` to ```i-th```

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

MarkMageeAstro commented on 2022-09-07T09:54:57Z ----------------------------------------------------------------

properties (specifically...

with respect to the x-axis...

used (along...

calculate the x and y...

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

MarkMageeAstro commented on 2022-09-07T09:54:58Z ----------------------------------------------------------------

Here the theme parameter...

Remove only

By default the light theme

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

MarkMageeAstro commented on 2022-09-07T09:54:59Z ----------------------------------------------------------------

Is there supposed to be something here? It doesn't show anything for me even in the docs preview

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

MarkMageeAstro commented on 2022-09-07T09:55:00Z ----------------------------------------------------------------

The Play button

The animation

to reach any

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

MarkMageeAstro commented on 2022-09-07T09:55:01Z ----------------------------------------------------------------

Does hovering need to be highlighted like this?

will show its properties

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

jaladh-singhal commented on 2022-09-07T15:04:34Z ----------------------------------------------------------------

Excellent! This was much needed

[atharva-2001]

@jayantbhakar could you please rerun these tests?

[jayantbhakar]

/azp run compare-refdata

[azure-pipelines[bot]]

Commenter does not have sufficient privileges for PR 2119 in repo tardis-sn/tardis

[andrewfullard]

/azp run compare-refdata

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[jamesgillanders]

We would like to resume this and get it merged. We need to investigate why some tests are failing -- any ideas?

[jamesgillanders]

@jayantbhakar Can you try rebasing this PR? There have been changes to the testing framework, and we would like to see if we can get these outstanding failing tests resolved.

[jamesgillanders]

@jayantbhakar Are you still able to work on this, or should we close the PR?

[jayantbhakar]

I am trying to rebase it and I am facing some conflicts in the file "docs/io/visualization/montecarlo_packet_visualization.ipynb". Would appreciate some help.

[jamesgillanders]

Hi Jayant, thanks for the reply. I will take a look soon and get back with some guidance!

[MarkMageeAstro]

Hi @jayantbhakar, what's the issue you're running into?

[jayantbhakar]

I am getting merge conflicts in the file "montecarlo_packet_visualization.ipynb". I don't think anyone else has worked on it, cuz this was related to my project.

[MarkMageeAstro]

There may have been some changes to some of the properties you were tracking due to the ongoing restructure work. What are the conflicts you're finding?

[jayantbhakar]

After resolving the conflics in rebasing, why are these tests failing now?

[MarkMageeAstro]

Hmm, I'm not very familiar with the tests, but it looks like it's something to do with the atomic data. Maybe it's changed? @andrewfullard might know

[andrewfullard]

Tests are failing due to changes in the structure of the code. We no longer have a simulation.runner object. Instead we have advance_state() and iterate() functions, and a simulation.simulation_state object. I think the simulation_state object is where the packet tracking information is located and is what should be tested against.

Docs are failing because of an intermittent error with loading the atom data. It's not caused by this PR.

[andrewfullard]

@jamesgillanders do you or @MarkMageeAstro have time to rebase this?

[andrewfullard]

Okay, I did the rebase so now it just needs some cleanup to the current code structure.

[jamesgillanders]

@jayantbhakar What is left for us to check/review with this PR? I haven't seen any updates since Andrew's rebasing -- did that resolve the issues you were facing, or is there more work needed?