-
Notifications
You must be signed in to change notification settings - Fork 19
Adding sysimage generation to mpi tests #48
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
Adding sysimage generation to mpi tests #48
Conversation
Codecov Report
@@ Coverage Diff @@
## release-0.2 #48 +/- ##
===========================================
Coverage 0.00% 0.00%
===========================================
Files 33 33
Lines 3205 3205
===========================================
Misses 3205 3205
Continue to review full report at Codecov.
|
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 not sure about this. Introduces considerable complexity and it has not a positive impact in the test times. In fact, by looking the gh actions it seems to even lead to slower times.
Perhaps separating the sequential and mpi tests is enough for now.
Yes, by now it does not improve much. If we want to perform tests with different values of the number of processors (apart from 1 and 4), this will be very helfpul. |
Besides, we will be evaluating the ability to precompile GridapDistributed.jl. Such action for clusters is a must. |
…into adding_sysimage_generation_to_mpi_tests
yes, but precompilation needs to be done in the final app, because we cannot anticipate which other packages is going to call the user. With PackageCompiler generating a system image is straight forward once you have the the final app. I think, it is not our job to provide a precompilaiton mechanism having PackageCompiler at our disposal. In any case, perhaps it is a good idea to create a new repo |
I think it makes a lot of sense to test the workflow that you are going to face when dealing with a cluster. The repo of apps is going to be abandoned, and not tested frequently (we have already many other experiences of similar repos). |
This is a very good point! but as the sysimage is created now, it is not testing the actual workflow that one needs to follow in a cluster. To check the workflow, you need an app, i.e., you need a (or some) "main" function(s) inside a Package and then create a sys image for those functions. We have several options:
I would go for 2 definitively, even though it has some more boilerplate code, since it actually mimics what a user needs to do in a cluster. What do you think? |
I agree the solution that we currently have in this branch is not definitive, in the sense that it is not testing exactly what will happen with the actual app, with all dependencies etc.. But it may happen that even with the current solution we detect a failure of precompilation after a commit. This can give us some early useful information, that would be otherwise very hard to obtain if the issue happens when you deploy the software in the cluster. |
and BTW the precompilation scrips should be in the "test app" (i.e. the final app) as usual |
Ok. Let us go for 2. Can we use the sysimage of the test app to accelerate the mpi tests or would you avoid that? |
* Addint automated image generation in CI.yml
…into adding_sysimage_generation_to_mpi_tests
@fverdugo ... all tests are now passing. PR ready to review/merge. The CI script generates the |
Solve Medium Priority Task in #39
IMPORTANT NOTE: This PR MUST BE reviewed after #47