Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GSoC] Velocity Packet Tracker Visualisation: First Objective #2539

Closed

Conversation

sarthak-dv
Copy link
Contributor

@sarthak-dv sarthak-dv commented Mar 10, 2024

📝 Description

Type: 🌞 GSoC

Solution to the first objective for the GSoC'24 project "Velocity Packet Tracker Visualisation". The Jupyter notebook contains the following tasks:

  • Run the example simulation and generate an SDEC plot.
  • Make a plot showing the abundance of each element, as a function of velocity.
  • Make a plot that illustrates the total number of interactions that escape the simulation from the different elements (I've only considered out line interactions here)

📌 Resources

GSoC'24 Page
TARDIS GSoC'24 Ideas Page

🚦 Testing

How did you test these changes?

  • Testing pipeline
  • Other method (describe): Ran the notebook and generated the plots
  • My changes can't be tested (explain why)

☑️ Checklist

  • I requested two reviewers for this pull request

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

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@sarthak-dv
Copy link
Contributor Author

@andrewfullard @jamesgillanders Please review

@sarthak-dv
Copy link
Contributor Author

Note to mentors: To make the virtual packet count work, I had to make a fix in MonteCarloTransportState.

I have raised an issue and the fix is in this PR

Copy link
Member

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting first objective @sarthak-dv

It's mostly looking good. It's great you caught a bug in the process and fixed it - now that your PR is merged, you can rebase this one.

It'll be also nice to see the plots in plotly since our tools use both matplotlib and plotly.

@sarthak-dv sarthak-dv force-pushed the gsoc24-vptv-first-objective branch from d7e1ece to ee98d1e Compare March 24, 2024 17:15
Print ion interaction count as a dataframe before plotting
Copy link

codecov bot commented Mar 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.18%. Comparing base (1718002) to head (4a60015).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2539   +/-   ##
=======================================
  Coverage   68.18%   68.18%           
=======================================
  Files         168      168           
  Lines       14219    14219           
=======================================
  Hits         9695     9695           
  Misses       4524     4524           

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

@sarthak-dv sarthak-dv force-pushed the gsoc24-vptv-first-objective branch from 2635656 to 1046e5a Compare March 24, 2024 17:56
@sarthak-dv
Copy link
Contributor Author

sarthak-dv commented Mar 24, 2024

Hi @jaladh-singhal

Thanks for the detailed review. I have made the changes acordingly (Added legend, plots in Plotly, printed the dataframes)
I've also rebased master into this, to bring in the bugfix.
Please check and let me know if more changes are required.

Thanks again!

@jamesgillanders
Copy link
Member

Hi Sarthak, looks good! Consider folding my comments above into the next iteration

Copy link
Member

@jamesgillanders jamesgillanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my recommended amendments from my attached comments

- Convert velocities to km/s
@sarthak-dv
Copy link
Contributor Author

Thanks for the review @jamesgillanders !

I have updated the plots with the following changes:

  • Convert yscale to "log"
  • Convert velocities to km/s

Since NBViewer doesn't render the Plotly plots, attaching screenshots for them here:

plotly_abundance

plotly_interactions

@sarthak-dv sarthak-dv deleted the gsoc24-vptv-first-objective branch August 22, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants