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

Removed nested explicit loop in Allen Cahn 2D FD exact solution #435

Merged
merged 3 commits into from
May 24, 2024

Conversation

brownbaerchen
Copy link
Contributor

Very minor performance improvement, a first baby step towards streamlining Allen-Cahn implementations as mentioned in #434.

Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.92%. Comparing base (18989d3) to head (e306b7b).
Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #435      +/-   ##
==========================================
+ Coverage   77.38%   77.92%   +0.53%     
==========================================
  Files         327      329       +2     
  Lines       26085    26133      +48     
==========================================
+ Hits        20187    20364     +177     
+ Misses       5898     5769     -129     

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

@brownbaerchen brownbaerchen added the girl scout rule All you did was leave the campground slightly cleaner than you found it. label May 22, 2024
@lisawim
Copy link
Collaborator

lisawim commented May 24, 2024

So few changes, but the code looks quite more cleaner now. This is nice! Using the np.meshgrid is this so much faster than using these two for loops?

@pancetta pancetta merged commit a549854 into Parallel-in-Time:master May 24, 2024
108 checks passed
@brownbaerchen
Copy link
Contributor Author

So few changes, but the code looks quite more cleaner now. This is nice! Using the np.meshgrid is this so much faster than using these two for loops?

In principle, explicit loops should be avoided in python in general. python is known for being slow compared to compiled languages. But this is no problem in practice, because we only use python to interface to compiled code with numpy etc, which uses shared memory parallelisation and vectorisation to speed up loops.
You can do that manually with explicit loops over lists when using numba, for jit compilation. But if you do neither, you get the very slow python speed. There are a lot of explicit loops in pySDC that are not compiled, but they are only small. For instance, we loop over the collocation nodes in the sweeper, which is just a handful of iterations. It doesn't make a large difference there and because of what goes on underneath, we can't use numpy in this case. And numba struggles with more complicated objects, as far as I know.

However, for the tests of this code, doesn't make a large difference. I guess the majority of the time is not spent in generating the initial conditions. However, it's bad practice and looks different from the mpi4py-fft version of Allen-Cahn. That's why I changed it, without really achieving too much :D

Maybe you saw me add the girl scout rule tag. With this I want to encourage exactly the sort of PR that this is. When you take the time to understand some piece of code, often you spot some way to quickly improve it in small ways. Then you can take an extra 30 mins and make it easier to understand for the next person reading it.

@brownbaerchen brownbaerchen deleted the AC_fix branch June 4, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
girl scout rule All you did was leave the campground slightly cleaner than you found it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants