Fix pre-compilation test, run unit tests with built system image #1123

jakebolewski · 2020-05-19T18:10:37Z

Drop test macro, just execute script. move into a sep build stage

I have

Written and run all necessary tests with CLIMA by including tests/runtests.jl
Followed all necessary style guidelines and run julia .dev/climaformat.jl .
Updated the documentation to reflect changes from this PR.

For review by CLIMA developers

There are no open pull requests for this already
CLIMA developers with relevant expertise have been assigned to review this submission
The code conforms to the style guidelines and has consistent naming conventions. julia .dev/format.jl has been run in a separate commit.
This code does what it is technically intended to do (all numerics make sense physically and/or computationally)

simonbyrne · 2020-05-19T18:12:14Z

azure-pipelines.yml

    displayName: 'Run the tests'
    continueOnError: true
+  - bash: |
+      set -o xtrace
+      ./julia-$(JULIA_VERSION)/bin/julia -e 'include(joinpath(".dev", "systemimage", "climate_machine_image.jl"))'


Should we just call the script?

Suggested change

./julia-$(JULIA_VERSION)/bin/julia -e 'include(joinpath(".dev", "systemimage", "climate_machine_image.jl"))'

./julia-$(JULIA_VERSION)/bin/julia .dev/systemimage/climate_machine_image.jl

yes probably, didn't know if we want to re-wrap this in a test macro (if this is determined to be the issue).

jakebolewski · 2020-05-19T21:25:55Z

didn't escape the execution string correctly, but it seems like this was the issue. I'll move ahead with getting the system image built and trying to use it for running the unit-tests.

simonbyrne · 2020-05-20T04:18:21Z

Disappointing the PackageCompiler.jl doesn't work on Windows:

C:/Users/VssAdministrator/.julia/artifacts/572b61b5075459e3ed62317e674398166ca98dd4/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: <unknown-file>:0: syntax error
Warning: .drectve `-export:ccalllib_C:\julia-1.3\bin\LLVM.dll,data ' unrecognized
C:/Users/VssAdministrator/.julia/artifacts/572b61b5075459e3ed62317e674398166ca98dd4/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: Cannot export ccalllib_C:: symbol not found
collect2.exe: error: ld returned 1 exit status

Possibly related to JuliaLang/PackageCompiler.jl#377?

I wonder if Julia 1.4 would help?

simonbyrne · 2020-05-20T04:26:44Z

It could be JuliaLang/julia#34680, which is supposed to be fixed in 1.4, or it could be JuliaLang/PackageCompiler.jl#365 / JuliaLang/julia#35039.

charleskawczynski · 2020-05-20T04:50:56Z

I’m a bit confused why we’re calling the sys image script in the azure yml. Isn’t this creating a sys image for a specific test file?

jakebolewski · 2020-05-20T14:55:10Z

@charleskawczynski Simon asked if this was possible to start introducing into the pipeline / tests / deployment, what it's actually pre-compiling at this point I don't know is that important (any code that exercises a decent amount of CliMA code will be sufficient, we can re-target what it's exactly pre-compiling to reduce overall average latency at a later point with some profiling).

@simonbyrne it looks like Julia 1.4 works on Windows! which is good news.

simonbyrne · 2020-05-20T16:29:38Z

@charleskawczynski The script used in the precompilation is just used as a template so it know what function signatures to build into the system image. We might want to do something more like use https://github.com/timholy/SnoopCompile.jl to generate a list of all statements we want to build into the tests. At the moment, we might want to use a script trim down the script used (we don't actually need it to do anything other than trigger the compilation path.

Lets switch to 1.4 on Windows for now, as that will unblock #1103 as well, then we can figure out the Slurm CI slowdowns later.

simonbyrne · 2020-05-20T17:37:51Z

before:

┌ Info: Running MPI test...
│   file = "/home/vsts/work/1/s/test/Numerics/Mesh/mpi_centroid.jl"
└   ntasks = 3
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
 54.380740 seconds (49.87 k allocations: 2.478 MiB)

after:

┌ Info: Running MPI test...
│   file = "/home/vsts/work/1/s/test/Numerics/Mesh/mpi_centroid.jl"
└   ntasks = 3
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
[ Info: CUDAdrv.jl failed to initialize, GPU functionality unavailable (set JULIA_CUDA_SILENT or JULIA_CUDA_VERBOSE to silence or expand this message)
  5.450521 seconds (49.87 k allocations: 2.477 MiB)

jkozdon · 2020-05-20T17:42:13Z

At the moment, we might want to use a script trim down the script used (we don't actually need it to do anything other than trigger the compilation path

Yeah what I did was a quick hack.

Also, if we are using it for testing, we may want to include ClimateMachine itself in the image. I didn't do this in what I did since I wanted to develop ClimateMachine itself.

.dev/systemimage/climate_machine_image.jl

jakebolewski · 2020-05-22T05:00:46Z

Updated

.dev/systemimage/climate_machine_image.jl

charleskawczynski · 2020-05-28T03:53:18Z

.dev/systemimage/climate_machine_image.jl

 end
+delete!(pkgs, "Pkg")


Why remove Pkg?

.dev/systemimage/climate_machine_image.jl

jakebolewski · 2020-05-29T01:46:29Z

It seems like there are issues with Pkg.dependencies() and our current Project.toml / Manifest file structure. I'm still getting used to how things work so maybe we should switch to defining a project structure that doesn't cause PkgCompiler to complain about missing dependencies. Pkg.installed() still works for collecting project packages for building a sys image that doesn't include ClimateMachine

jakebolewski · 2020-06-01T16:17:35Z

@simonbyrne is this ready to merge?

simonbyrne

Just need to resolve the conflict.

1177: Move remaining Mesh tests to runmpi structure r=simonbyrne a=jakebolewski # Description Move remaining Numerics / Mesh tests to use `runmpi` test harness. Should be sped up by #1123. closes #1168 I have - [x] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [x] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. Co-authored-by: jakebolewski <[email protected]>

simonbyrne · 2020-06-01T16:46:52Z

bors r+

1123: Fix pre-compilation test, run unit tests with built system image r=simonbyrne a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. Co-authored-by: jakebolewski <[email protected]>

bors · 2020-06-01T20:25:43Z

Canceled.

jakebolewski · 2020-06-01T21:06:19Z

bors r+

1123: Fix pre-compilation test, run unit tests with built system image r=jakebolewski a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. Co-authored-by: jakebolewski <[email protected]>

bors · 2020-06-01T23:40:40Z

Build failed:

azure-ci

jakebolewski · 2020-06-02T21:13:05Z

Needs to be rebased on #1185

* add option to compile climatemachine package into sysimg * update ci to use pre-compiled system iamge * bump pipeline def. to use julia 1.4

jakebolewski · 2020-06-03T15:50:28Z

bors r+

@tapios

1103: update CuArrays to 2.2 r=simonbyrne a=simonbyrne # Description Fixes #1044, supercedes #1073 and #1099. 1123: Fix pre-compilation test, run unit tests with built system image r=jakebolewski a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. 1172: Separate cli arg parsing from initialization r=jakebolewski a=jakebolewski # Description Separates out cli argument processing from `ClimateMachine.init()` by defining a new method `ClimateMachine.cli()` that parses arguments and initializes ClimateMachine runtime. `ClimateMachine.init()` is preserved to allow for programmatic initialization when command line parsing is not needed (tests / notebooks / scripts). `ClimateMachine.cli` and `ClimateMachine.init` now take keyword arguments to allow for easy overloading for `ClimateMachine.Settings` at configuration / runtime. The experiment files have been updated to use `cli`, the tutorials I kept the same. Consistent overloading fallback behaviour is also defined: 1. cli arguments (if applicable) 2. keyword argument overloads 3. `CLIMATEMACHINE_SETTINGS_<VALUE_NAME>` process environment definitions 4. `ClimateMachine.Settings` default values. This allows for easy overloading of settings in batch scripts / container environments without having to define configuration files or change the driver file source code. We were currently doing this in kind of an add-hoc fashion. Closes #1167 1187: remove MPI installation from Documenter and Coverage r=simonbyrne a=simonbyrne # Description This is no longer necessary. 1192: Add comments to temperature profiles tests r=charleskawczynski a=charleskawczynski # Description - Adds some comments explaining temperature profiles tests Co-authored with: @tapios Co-authored-by: Simon Byrne <[email protected]> Co-authored-by: jakebolewski <[email protected]> Co-authored-by: Charles Kawczynski <[email protected]>

bors · 2020-06-04T04:00:01Z

This PR was included in a batch that was canceled, it will be automatically retried

@tapios

1123: Fix pre-compilation test, run unit tests with built system image r=jakebolewski a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. 1172: Separate cli arg parsing from initialization r=jakebolewski a=jakebolewski # Description Separates out cli argument processing from `ClimateMachine.init()` by defining a new method `ClimateMachine.cli()` that parses arguments and initializes ClimateMachine runtime. `ClimateMachine.init()` is preserved to allow for programmatic initialization when command line parsing is not needed (tests / notebooks / scripts). `ClimateMachine.cli` and `ClimateMachine.init` now take keyword arguments to allow for easy overloading for `ClimateMachine.Settings` at configuration / runtime. The experiment files have been updated to use `cli`, the tutorials I kept the same. Consistent overloading fallback behaviour is also defined: 1. cli arguments (if applicable) 2. keyword argument overloads 3. `CLIMATEMACHINE_SETTINGS_<VALUE_NAME>` process environment definitions 4. `ClimateMachine.Settings` default values. This allows for easy overloading of settings in batch scripts / container environments without having to define configuration files or change the driver file source code. We were currently doing this in kind of an add-hoc fashion. Closes #1167 1187: remove MPI installation from Documenter and Coverage r=simonbyrne a=simonbyrne # Description This is no longer necessary. 1192: Add comments to temperature profiles tests r=charleskawczynski a=charleskawczynski # Description - Adds some comments explaining temperature profiles tests Co-authored with: @tapios Co-authored-by: jakebolewski <[email protected]> Co-authored-by: Simon Byrne <[email protected]> Co-authored-by: Charles Kawczynski <[email protected]>

bors · 2020-06-04T06:46:09Z

Build failed (retrying...):

ci/slurmci

1123: Fix pre-compilation test, run unit tests with built system image r=jakebolewski a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. 1172: Separate cli arg parsing from initialization r=jakebolewski a=jakebolewski # Description Separates out cli argument processing from `ClimateMachine.init()` by defining a new method `ClimateMachine.cli()` that parses arguments and initializes ClimateMachine runtime. `ClimateMachine.init()` is preserved to allow for programmatic initialization when command line parsing is not needed (tests / notebooks / scripts). `ClimateMachine.cli` and `ClimateMachine.init` now take keyword arguments to allow for easy overloading for `ClimateMachine.Settings` at configuration / runtime. The experiment files have been updated to use `cli`, the tutorials I kept the same. Consistent overloading fallback behaviour is also defined: 1. cli arguments (if applicable) 2. keyword argument overloads 3. `CLIMATEMACHINE_SETTINGS_<VALUE_NAME>` process environment definitions 4. `ClimateMachine.Settings` default values. This allows for easy overloading of settings in batch scripts / container environments without having to define configuration files or change the driver file source code. We were currently doing this in kind of an add-hoc fashion. Closes #1167 Co-authored-by: jakebolewski <[email protected]>

bors · 2020-06-04T09:11:09Z

Build failed (retrying...):

ci/slurmci

1123: Fix pre-compilation test, run unit tests with built system image r=jakebolewski a=jakebolewski Drop test macro, just execute script. move into a sep build stage I have - [ ] Written and run all necessary tests with CLIMA by including `tests/runtests.jl` - [ ] Followed all necessary [style guidelines](https://CliMA.github.io/CLIMA/latest/CodingConventions.html) and run `julia .dev/climaformat.jl .` - [ ] Updated the documentation to reflect changes from this PR. Co-authored-by: jakebolewski <[email protected]>

bors · 2020-06-04T13:50:59Z

Build failed:

ci/slurmci

jakebolewski · 2020-06-04T20:01:08Z

bors r+

bors · 2020-06-04T22:09:03Z

Build succeeded:

simonbyrne reviewed May 19, 2020

View reviewed changes

jakebolewski changed the title ~~Don't use single quote for cmd.exe, escape double quotes~~ Fix pre-compilation test, run unit tests with built system image May 20, 2020

jakebolewski mentioned this pull request May 20, 2020

Support Julia 1.4? #1061

Closed

3 tasks

simonbyrne reviewed May 20, 2020

View reviewed changes