Skip to content

power spectrum module optimization and parallelization#102

Merged
lgarrison merged 8 commits intomainfrom
pk_bench
Jul 14, 2023
Merged

power spectrum module optimization and parallelization#102
lgarrison merged 8 commits intomainfrom
pk_bench

Conversation

@lgarrison
Copy link
Member

Some optimization, parallelization, and Numba-fication of various parts of the power spectrum module. Benchmark scripts used to produce the timings in the ZCV paper are included.

@lgarrison lgarrison marked this pull request as draft July 12, 2023 17:39
@lgarrison
Copy link
Member Author

@boryanah Here are the changes we talked about. It's about half optimization and half refactoring. I tried to rename some things that were confusing to me, but let me know if I misunderstood anything. I also tried to cut down on the number of arguments and return values in some places. For example, I changed calc_power() to return an Astropy Table; let me know if you think that makes sense.

@lgarrison lgarrison marked this pull request as ready for review July 12, 2023 22:05
@lgarrison lgarrison requested a review from boryanah July 12, 2023 22:07
@boryanah
Copy link
Collaborator

That looks great to me! The optimizations make sense (I am sorry I didn't implement the normalization one myself). The astropy table for the power spectrum with the meta data is great, and I think it's a good solution for outputting a single object that contains some useful information about the simulation. I think it makes sense why you got rid of some of the del X; gc.collect() (they probably weren't doing anything as the variables were passed externally rather than locally defined). I also like the variable and function name changes, and I think they make more sense.

@lgarrison
Copy link
Member Author

Yeah, that's exactly right about the gc.collect(). The other reason was that garbage collection was making the timings really noisy; there might be one or two that could go back in, but for the most part I think they weren't doing anything. And if we really wanted to save memory, there are other ways: using an in-place FFT (pyfftw supports this, not sure if scipy.fft does), and adding on-the-fly offsets for the interlacing calculation (right now it makes a whole copy of the input data).

@lgarrison lgarrison merged commit b0ae7b7 into main Jul 14, 2023
@lgarrison lgarrison deleted the pk_bench branch July 14, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants