-
Notifications
You must be signed in to change notification settings - Fork 6
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
Allow using P4est
for preferences not set correctly
#93
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #93 +/- ##
==========================================
+ Coverage 16.18% 16.25% +0.08%
==========================================
Files 3 3
Lines 1533 1538 +5
==========================================
+ Hits 248 250 +2
- Misses 1285 1288 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering: Right now, the preferences check happens at the module level, i.e., during precompilation, if I am not mistaken. Wouldn't it be better if this warning were shown (also) at loading time, i.e., from __init__
?
Also, this will lead to no variable libp4est
/libsc
being defined in the module - is this intentional?
Yes, definitely. I added that in 17d4a75.
Yes, this is intentional. If MPIPreferences are set, but no P4est.jl preferences, we can neither set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! @sloede Do you want to have a final look as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
With this PR
using P4est
doesn't crash anymore if a system-provided MPI installation together with the P4est_jll p4est version is used, but gives the user a warning to set the preferences for p4est accordingly. This way, it is also possible to first set the MPIPreferences, restart the REPL and then set the preferences for P4est.jl.Additionally, I have added three functions:
path_p4est_library()
andpath_sc_library()
, which returns the path to the libraries orP4est_jll
if the default p4est installation is used. This way one can conveniently check if P4est.jl uses the correct libraries.Lastly, I added the function
preferences_set_correctly()
, which returnsfalse
if P4est.jl is not usable due to missing preference for P4est.jl. This function can be used in Trixi.jl to determine ifinit_p4est()
can be called or not. By addingif preferences_set_correctly() ... end
together with my corresponding PR in T8code.jl (DLR-AMR/T8code.jl#40), I was able to run Trixi.jl with system-provided MPI installation on aTreeMesh
-example without setting the preferences for P4est.jl and T8code.jl. Thus, with these two PRs and the corresponding PR in Trixi.jl (that I will prepare once this PR and the corresponding T8code.jl PR are merged) it is only necessary to set the preferences for P4est.jl and T8code.jl if you really need them.Closes #91.