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

restructure of rpacket_tracker #2714

Closed
wants to merge 12 commits into from

Conversation

Sumit112192
Copy link
Contributor

📝 Description

Type: 🎢 infrastructure and restructure

This PR restructures the rpacket_tracker code to initialize only when the track_rpacket flag is set. Apart from that, I will also add tests for the rpacket_tracker and change how it registers an interaction.

@Sumit112192 Sumit112192 changed the title This PR intends to use global variables for the rpacket_tracker restructure of rpacket_tracker Jul 17, 2024
@Sumit112192 Sumit112192 marked this pull request as ready for review July 17, 2024 11:46
@Sumit112192
Copy link
Contributor Author

@wkerzendorf @andrewfullard Please review.

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

Attention: Patch coverage is 32.60870% with 62 lines in your changes missing coverage. Please review.

Project coverage is 36.49%. Comparing base (c107f27) to head (1368603).

Files Patch % Lines
...transport/montecarlo/tests/test_rpacket_tracker.py 38.09% 39 Missing ⚠️
...dis/visualization/tools/tests/test_rpacket_plot.py 33.33% 10 Missing ⚠️
...ardis/transport/montecarlo/montecarlo_main_loop.py 0.00% 6 Missing ⚠️
tardis/transport/montecarlo/single_packet_loop.py 14.28% 6 Missing ⚠️
tardis/visualization/tools/rpacket_plot.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2714      +/-   ##
==========================================
+ Coverage   36.47%   36.49%   +0.02%     
==========================================
  Files         183      183              
  Lines       14664    14685      +21     
==========================================
+ Hits         5348     5359      +11     
- Misses       9316     9326      +10     

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

@Sumit112192
Copy link
Contributor Author

Profiling:
Master Branch:
image
FixRPacketTracker Branch:
image
Memory usage has gone done.

-------
simulation object with track_rpacket enabled
"""
config_rpacket_tracking.montecarlo.iterations = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need 3 iterations for the test to be useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. But it shouldn't take too much time.

@Sumit112192
Copy link
Contributor Author

Completet as PR #2716 is merged

@Sumit112192 Sumit112192 deleted the FixRPacketTracker branch September 8, 2024 13:07
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.

2 participants