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

Julia script creation system custom images #436

Closed

Conversation

amartinhuertas
Copy link
Member

@amartinhuertas amartinhuertas commented Nov 2, 2020

Note: this PR is highly related to #432

@codecov-io
Copy link

codecov-io commented Nov 3, 2020

Codecov Report

Merging #436 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #436   +/-   ##
=======================================
  Coverage   87.34%   87.34%           
=======================================
  Files         158      158           
  Lines       11170    11170           
=======================================
  Hits         9756     9756           
  Misses       1414     1414           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c8f6a5...70b6c33. Read the comment docs.

@amartinhuertas
Copy link
Member Author

Hi @fverdugo,

I have already done the following in this PR:

You want to create and use the system image in the 3rd job in the test stage, right?
There is something that is problematic though. The default sys image creation depends on Tutorials and Tutorials depends on Gridap... It will cause some trouble when developing braking releases... Perhaps in travis, we want to generate the sys image by snooping the tests in GridapTests instead of cloning Tutorials.

However, I get the following error while executing the system image generation script on Travis:

https://travis-ci.com/github/gridap/Gridap.jl/jobs/425860847#L318

I could confirm that the error does not arise on my local machine with Julia 1.4.0, but it does with Julia 1.5.2! (the one that is being used for the travis job that fails).

The error seems to be quite cryptic to me, it is like some kind of low-level error while executing LLVM code. Does the error message tell you something? If not, I can try to open an issue either on PackageCompiler.jl or the main Julia repo. What do u think?

@fverdugo
Copy link
Member

fverdugo commented Nov 3, 2020

Hi @amartinhuertas I have never seen this error...

However, I find some very weird error from time to time, perhaps it is a manifestation of the same problem. See this issue #400 (you can go straight to my first comment).

@amartinhuertas
Copy link
Member Author

However, I find some very weird error from time to time, perhaps it is a manifestation of the same problem. See this issue #400 (you can go straight to my first comment).

Yes, I was aware about this akward error ... Indeed, I also though that this might be behind this particular error. However, today I have discovered that the following command:

julia compile/create_gridap_image.jl --do-not-clone-gridap --gridap-path . --user-provided-script-path ../Tutorials/src/stokes.jl -n ./Gridap.so

did not fail. For completeness, I am using the stokes.jl Tutorial, release tag v0.13.0.

The command that fails (reproducibly) is:

julia compile/create_gridap_image.jl --do-not-clone-gridap --gridap-path . --user-provided-script-path test/GridapTests/PoissonTests.jl -n ./Gridap.so

So perhaps is something in test/GridapTests/PoissonTests.jl ... not sure at the current moment. Just documenting what I have learned so far here.

@amartinhuertas
Copy link
Member Author

I have been able to reproduce the error with a different test (StokesDGTests.jl). Thus, there seems to be nothing particular in test/GridapTests/PoissonTests.jl.

julia compile/create_gridap_image.jl --do-not-clone-gridap --gridap-path . --user-provided-script-path test/GridapTests/StokesDGTests.jl -n ./Gridap.so
[ Info: PackageCompiler: creating system image object file, this might take a while...
Invalid bitcast
  %45 = bitcast %jl_value_t addrspace(10)* %39 to i64, !dbg !432327
in function julia_common_size_98862
LLVM ERROR: Broken function found, compilation aborted!
ERROR: LoadError: failed process: Process(`/home/amartin/software_installers/julia-1.5.2-linux-x86_64/julia-1.5.2/bin/julia --color=yes --startup-file=no --cpu-target=native --sysimage=/home/amartin/software_installers/julia-1.5.2-linux-x86_64/julia-1.5.2/lib/julia/sys.so --project=/home/amartin/git-repos/Gridap.jl --output-o=/tmp/jl_Oygvlj.o -e 'Base.reinit_stdio()
@eval Sys BINDIR = ccall(:jl_get_julia_bindir, Any, ())::String
Base.init_load_path()
Base.init_depot_path()
import Gridap
import LineSearches
import SparseArrays
import NLsolve
import DocStringExtensions
import QuadGK
import FileIO
import JSON
import WriteVTK
import LinearAlgebra
import JLD2
import StaticArrays
import Combinatorics
import AbstractTrees
import ForwardDiff
import Test
import BSON
import FastGaussQuadrature
import FillArrays
import BlockArrays
# This @eval prevents symbols from being put into Main
@eval Module() begin
    PrecompileStagingArea = Module()
    for (_pkgid, _mod) in Base.loaded_modules
        if !(_pkgid.name in ("Main", "Core", "Base"))
            eval(PrecompileStagingArea, :(const $(Symbol(_mod)) = $_mod))
        end
    end
    precompile_statements = String[]
        append!(precompile_statements, readlines("/tmp/jl_npBesh"))

    for statement in sort(precompile_statements)
        # println(statement)
        # The compiler has problem caching signatures with \`Vararg{?, N}\`. Replacing
        # N with a large number seems to work around it.
        statement = replace(statement, r"Vararg{(.*?), N} where N" => s"Vararg{\1, 100}")
        try
            Base.include_string(PrecompileStagingArea, statement)
        catch
            # See julia issue #28808
            @debug "failed to execute $statement"
        end
    end
end # module
empty!(LOAD_PATH)
empty!(DEPOT_PATH)
'`, ProcessExited(1)) [1]

Stacktrace:
 [1] pipeline_error at ./process.jl:525 [inlined]
 [2] run(::Cmd; wait::Bool) at ./process.jl:440
 [3] run at ./process.jl:438 [inlined]
 [4] create_sysimg_object_file(::String, ::Array{String,1}; project::String, base_sysimage::String, precompile_execution_file::Array{String,1}, precompile_statements_file::Array{String,1}, cpu_target::String, script::Nothing, isapp::Bool) at /home/amartin/.julia/packages/PackageCompiler/AerNj/src/PackageCompiler.jl:295
 [5] create_sysimage(::Array{Symbol,1}; sysimage_path::String, project::String, precompile_execution_file::String, precompile_statements_file::Array{String,1}, incremental::Bool, filter_stdlibs::Bool, replace_default::Bool, cpu_target::String, script::Nothing, base_sysimage::Nothing, isapp::Bool) at /home/amartin/.julia/packages/PackageCompiler/AerNj/src/PackageCompiler.jl:426
 [6] main() at /home/amartin/git-repos/Gridap.jl/compile/create_gridap_image.jl:148
 [7] top-level scope at /home/amartin/git-repos/Gridap.jl/compile/create_gridap_image.jl:159
 [8] include(::Function, ::Module, ::String) at ./Base.jl:380
 [9] include(::Module, ::String) at ./Base.jl:368
 [10] exec_options(::Base.JLOptions) at ./client.jl:296
 [11] _start() at ./client.jl:506
in expression starting at /home/amartin/git-repos/Gridap.jl/compile/create_gridap_image.jl:159

@fverdugo
Copy link
Member

fverdugo commented Nov 4, 2020

@amartinhuertas It seems a tricky error!

since Gridap is going to change A LOT (see wip in branch kernel_refactoring), I would suggest to stop investigating this for now. We can try to make it work once the new version is ready.

@amartinhuertas
Copy link
Member Author

ok, agreed.

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #436 (081ede5) into master (33dea3c) will decrease coverage by 87.98%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #436       +/-   ##
==========================================
- Coverage   87.98%   0.00%   -87.99%     
==========================================
  Files         133     132        -1     
  Lines       14237   14506      +269     
==========================================
- Hits        12527       0    -12527     
- Misses       1710   14506    +12796     
Impacted Files Coverage Δ
src/Io/Bson.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Io/Json.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Arrays/Autodiff.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Arrays/ArrayPairs.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Arrays/SubVectors.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Arrays/AlgebraMaps.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Geometry/GridMocks.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Helpers/GridapTypes.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Fields/InverseFields.jl 0.00% <0.00%> (-100.00%) ⬇️
src/Arrays/FilteredArrays.jl 0.00% <0.00%> (-100.00%) ⬇️
... and 119 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33dea3c...081ede5. Read the comment docs.

@amartinhuertas
Copy link
Member Author

@amartinhuertas It seems a tricky error!
since Gridap is going to change A LOT (see wip in branch kernel_refactoring), I would suggest to stop investigating this for now. We can try to make it work once the new version is ready.

@fverdugo ... I could make it work without issues with Gridap 0.16.0 + Julia 16.

Thus, I guess that this PR is (almost) ready to be merged.

The only pending decision is whether PoissonTests.jl is the most appropriate source file from which to create the image in the CI system. (It takes 837 secs. approx. to create the image.)

Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @amartinhuertas !

I have been thinking about this and I have some comments

  1. The approach seems a bit complicated to me. In particular, with the clones of Gridap, Gridap Tutorials. It feels like we are trying to re-implement something that the package manager already provides.
  2. Wrapping calls to PackageCompiler is a bit rigid and one needs to learn the API of the wrapper. I would prefer to use PackageCompiler directly. It is a well documented package and once the user knows how to use it, it can be useful for other Julia projects, not just Gridap.
  3. This approach is vulnerable to code invalidation. When the user imports packages that were not present at this point, some code generated in the image can be invalidated.

In summary:

In Gridap, I would only provide a path to a "warm-up file", or some function returning a path in function of some arguments if we want some customization. Then, the user can use this warm-up file to call PackageCompiler in the way she/he needs for the case at hand.

Some tentative user code

using PackageCompiler
using Gridap
# load other packages that the user wants to use to avoid code invalidations
using Plots
using DrWatson
using GridapEmbedded

# Recover the warmup file for the Gridap version installed
# (package manager will handle the installation of Gridap)
warmup_gridap = Gridap.warmup_file

# Here one has the oportunity to compile more things
pkgs = [:Gridap,:OtherPackage1,...]
warmups= [warmup_gridap, other_warmup, ....]

 PackageCompiler.create_sysimage(
pkgs,
sysimage_path="whatever.so",
precompile_execution_file=warmups)

@amartinhuertas
Copy link
Member Author

In Gridap, I would only provide a path to a "warm-up file", or some function returning a path in function of some arguments if we want some customization. Then, the user can use this warm-up file to call PackageCompiler in the way she/he needs for the case at hand.

Ok, I agree this makes sense for Gridap. I will thus add the script to GridapDistributed (it is helpful there in the meantime) and close this PR. I have created issue #630 to not forget on these ideas.

@amartinhuertas amartinhuertas deleted the julia_script_creation_system_custom_images branch August 19, 2021 06:46
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.

4 participants