Restructure Trackers

[Sumit112192]

📝 Description

Type: 🎢 infrastructure
This PR brings the rpacket_trackers initialization outside the montecarlo_main_loop to avoid the usage of compile time constants and reduce complexity.

[codecov]

Codecov Report

Attention: Patch coverage is 74.35897% with 10 lines in your changes missing coverage. Please review.

Project coverage is 69.30%. Comparing base (bcbe40d) to head (55b291e).
Report is 7 commits behind head on master.

Files	Patch %	Lines
tardis/transport/montecarlo/packet_trackers.py	43.75%	9 Missing ⚠️
...ardis/transport/montecarlo/montecarlo_main_loop.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2719      +/-   ##
==========================================
- Coverage   69.65%   69.30%   -0.36%     
==========================================
  Files         188      192       +4     
  Lines       14930    15047     +117     
==========================================
+ Hits        10400    10428      +28     
- Misses       4530     4619      +89     

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

[tardis-bot]

*beep* *bop*
Hi human,
I ran benchmarks as you asked comparing master (bcbe40d) and the latest commit (c18503a).
Here are the logs produced by ASV.
Results can also be downloaded as artifacts here.

Significantly changed benchmarks:

· Did not find results for commit c18503a794babddd9b54e1e3aaa89f733ced4e41

All benchmarks:

· Did not find results for commit c18503a794babddd9b54e1e3aaa89f733ced4e41

If you want to see the graph of the results, you can check it here

[Sumit112192]

Profiling: Though the time taken fluctuates. Take +- 30s
Master Branch:

RestructureRPacketTrackers Branch:

[andrewfullard]

So you are just using pure Python to initialize the lists and passing them along to the main loop?

[Sumit112192]

Yes. That's the only way it could be initialized without dead branch pruning.

[Sumit112192]

I could move the initialization to montecarlo_globals with numba compilation. Should I go for that?

[andrewfullard]

Need to test the speed of the initialization

[Sumit112192]

Testing the speed of appending to a numba list in pure python function and function decorated with Numba.

import numba
from numba.typed import List
import timeit

from tardis.transport.montecarlo.packet_trackers import RPacketTracker, RPacketLastInteractionTracker

no_of_packets = 100000

@numba.njit
def return_rpacket_tracker():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketTracker(10))
    return aList

@numba.njit
def return_rpacket_last_interaction_tracker():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketLastInteractionTracker())
    return aList

def return_rpacket_tracker_pure():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketTracker(10))
    return aList

def return_rpacket_last_interaction_tracker_pure():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketLastInteractionTracker())
    return aList

%timeit return_rpacket_tracker()

237 ms ± 3.23 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%timeit return_rpacket_last_interaction_tracker()

24 ms ± 753 μs per loop (mean ± std. dev. of 7 runs, 1 loop each)

%timeit return_rpacket_tracker_pure()

1.22 s ± 22.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%timeit return_rpacket_last_interaction_tracker_pure()

794 ms ± 13.6 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

[Sumit112192]

Testing the speed of various ways to solve the current problem.

from numba.typed import List
import numba
import timeit

from tardis.transport.montecarlo.packet_trackers import RPacketTracker, RPacketLastInteractionTracker

no_of_packets = 100000

appending in true python

def aNormalFunc(flag):
    aList = List()
    if flag:
        for i in range(no_of_packets):
            aList.append(RPacketTracker(10))
    else:
        for i in range(100000):
            aList.append(RPacketLastInteractionTracker())
    return aList

appending in numba

flag = True
@numba.njit
def aNumbaFunc():
    aList = List()
    if flag:
        for i in range(no_of_packets):
            aList.append(RPacketTracker(10))
    else:
        for i in range(no_of_packets):
            aList.append(RPacketLastInteractionTracker())
    return aList

appending in true python with loop in numba

@numba.njit
def returnRPacketTracker():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketTracker(10))
    return aList

@numba.njit
def returnRPacketLastInteractionTracker():
    aList = List()
    for i in range(no_of_packets):
        aList.append(RPacketLastInteractionTracker())
    return aList

def aNormalFuncWithNumbaLoop(flag):
    if flag:
        aList = returnRPacketTracker()
    else:
        aList = returnRPacketLastInteractionTracker()
    return aList

Checking runtime using timeit

%timeit aNormalFunc(True)

1.27 s ± 110 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%timeit aNumbaFunc()

266 ms ± 22.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

%timeit aNormalFuncWithNumbaLoop(True);

243 ms ± 12.3 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

[wkerzendorf]

@atharva-2001 @andrewfullard I think this means we do not need a separate RPacketTracker test suite

[wkerzendorf]

@Sumit112192 please add tests

[tardis-bot]

*beep* *bop*
Hi human,
I ran ruff on the latest commit (55b291e).
Here are the outputs produced.
Results can also be downloaded as artifacts here.
Summarised output:

Complete output(might be large):

[Sumit112192]

Regarding the time usage of the random number generator

import numpy as np

no_of_packets = 50

%timeit np.random.randint(0, no_of_packets)

2.89 μs ± 199 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

[Sumit112192]

@Sumit112192 please add tests

Done.

[Sumit112192]

I have added benchmarks for the packet_tracker utility functions, namely, time_generate_rpacket_tracker_list and time_generate_rpacket_last_interaction_tracker_list.
It's helpful if, in the future, someone adds some attributes to the tracker classes, which might slow it down so the utility benchmark can capture it.

[Sumit112192]

Also, I am parameterizing the benchmarks the way they are given in the documentation as opposed to the parameterized marker, i.e.,
time_benchmark_name.params = ['some_iterable1', 'some_iterable2']
time_benchmark_name.param_names = ['name1', 'name2']
I found that to be easy to follow.

[andrewfullard]

Happy to merge this if the benchmarks running can be confirmed

[Sumit112192]

Happy to merge this if the benchmarks running can be confirmed

The benchmarks pass if we look at the first 50% of the GitHub benchmark check. It fails on the master branch commit because the utility functions are not currently available in the master branch.