Skip to content

Comments

test CI on Julia v1.12#216

Merged
JoshuaLampert merged 20 commits intomainfrom
test-1.12
Oct 9, 2025
Merged

test CI on Julia v1.12#216
JoshuaLampert merged 20 commits intomainfrom
test-1.12

Conversation

@JoshuaLampert
Copy link
Member

@JoshuaLampert JoshuaLampert commented Jun 7, 2025

Problems:

  • │ WARNING: Constructor for type "Int16" was extended inVectorizationBase without explicit qualification or import. from VectorizationBase.jl, fixed by a new release of VectorizationBase.jl.
  • Deprecations from JuliaSIMD packages (cf. Precompilation fails on julia master due to llvmcall depreacation JuliaSIMD/ThreadingUtilities.jl#51), fixed for ThreadingUtilities.jl and now also VectorizationBase.jl , but there are still warnings from LoopVectorization.jl and also SummationByPartsOperators.jl (probably also due to LoopVectorization.jl):
│  WARNING: llvmcall with integer pointers is deprecated.
│  Use actual pointers instead, replacing i32 or i64 with i8* or ptr
Failed to precompile JET [c3a54625-cd67-489e-a8e7-0a5a0ff4e31b] to "/home/runner/.julia/compiled/v1.12/JET/jl_JlY6QG".
WARNING: Imported binding Compiler.istopfunction was undeclared at import time during import to JET.
ERROR: LoadError: UndefVarError: `LineInfoNode` not defined in `JET`

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2025

Benchmark Results (Julia v1.10)

Time benchmarks
main e1605ec... main / e1605ec...
bbm_1d/bbm_1d_basic.jl - rhs!: 14.4 ± 0.55 μs 14.4 ± 0.55 μs 0.995 ± 0.054
bbm_1d/bbm_1d_fourier.jl - rhs!: 0.529 ± 0.0095 ms 0.221 ± 0.31 ms 2.4 ± 3.4
bbm_bbm_1d/bbm_bbm_1d_basic_reflecting.jl - rhs!: 0.0807 ± 0.00023 ms 0.082 ± 0.00038 ms 0.984 ± 0.0054
bbm_bbm_1d/bbm_bbm_1d_dg.jl - rhs!: 0.0343 ± 0.00075 ms 0.0342 ± 0.00053 ms 1 ± 0.027
bbm_bbm_1d/bbm_bbm_1d_relaxation.jl - rhs!: 27.1 ± 0.45 μs 28.6 ± 0.89 μs 0.949 ± 0.034
bbm_bbm_1d/bbm_bbm_1d_upwind_relaxation.jl - rhs!: 0.0485 ± 0.00076 ms 0.049 ± 0.00061 ms 0.992 ± 0.02
hyperbolic_serre_green_naghdi_1d/hyperbolic_serre_green_naghdi_dingemans.jl - rhs!: 4.25 ± 0.03 μs 4.32 ± 0.04 μs 0.984 ± 0.011
kdv_1d/kdv_1d_basic.jl - rhs!: 1.46 ± 0.011 μs 1.51 ± 0.019 μs 0.967 ± 0.014
kdv_1d/kdv_1d_implicit.jl - rhs!: 1.41 ± 0.01 μs 1.41 ± 0.01 μs 1 ± 0.01
serre_green_naghdi_1d/serre_green_naghdi_well_balanced.jl - rhs!: 0.198 ± 0.0084 ms 0.205 ± 0.01 ms 0.967 ± 0.063
svaerd_kalisch_1d/svaerd_kalisch_1d_dingemans_relaxation.jl - rhs!: 0.147 ± 0.0039 ms 0.149 ± 0.0064 ms 0.989 ± 0.05
time_to_load 1.98 ± 0.016 s 2.07 ± 0.0023 s 0.96 ± 0.0079
Memory benchmarks
main e1605ec... main / e1605ec...
bbm_1d/bbm_1d_basic.jl - rhs!: 1 allocs: 4.12 kB 1 allocs: 4.12 kB 1
bbm_1d/bbm_1d_fourier.jl - rhs!: 1 allocs: 4.12 kB 1 allocs: 4.12 kB 1
bbm_bbm_1d/bbm_bbm_1d_basic_reflecting.jl - rhs!: 5 allocs: 1.17 kB 5 allocs: 1.17 kB 1
bbm_bbm_1d/bbm_bbm_1d_dg.jl - rhs!: 10 allocs: 8.62 kB 10 allocs: 8.62 kB 1
bbm_bbm_1d/bbm_bbm_1d_relaxation.jl - rhs!: 2 allocs: 8.25 kB 2 allocs: 8.25 kB 1
bbm_bbm_1d/bbm_bbm_1d_upwind_relaxation.jl - rhs!: 2 allocs: 8.25 kB 2 allocs: 8.25 kB 1
hyperbolic_serre_green_naghdi_1d/hyperbolic_serre_green_naghdi_dingemans.jl - rhs!: 0 allocs: 0 B 0 allocs: 0 B
kdv_1d/kdv_1d_basic.jl - rhs!: 0 allocs: 0 B 0 allocs: 0 B
kdv_1d/kdv_1d_implicit.jl - rhs!: 0 allocs: 0 B 0 allocs: 0 B
serre_green_naghdi_1d/serre_green_naghdi_well_balanced.jl - rhs!: 0.075 k allocs: 0.66 MB 0.075 k allocs: 0.66 MB 1
svaerd_kalisch_1d/svaerd_kalisch_1d_dingemans_relaxation.jl - rhs!: 0.042 k allocs: 0.315 MB 0.042 k allocs: 0.315 MB 1
time_to_load 0.153 k allocs: 14.5 kB 0.153 k allocs: 14.5 kB 1

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2025

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/solver.jl 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@ranocha
Copy link
Member

ranocha commented Aug 25, 2025

JET.jl fails to precompile. I think JET.jl is only compatible with Julia v1.12 in newer versions than we currently allow (because newer versions are incompatible with Julia v1.10 and v1.11, cf. CompatHelper: bump compat for JET to 0.10 for package test, (keep existing compat) #194). So I it will probably be hard to let the tests run on both pre-1.12 and 1.12.

Would it make sense to move the JET.jl tests to a new CI job - or multiple of them - to fix this? Alternatively, it seems to be fine to use JET.jl only with a specific version, e.g., the LTS or the newest release.

@JoshuaLampert
Copy link
Member Author

JET.jl fails to precompile. I think JET.jl is only compatible with Julia v1.12 in newer versions than we currently allow (because newer versions are incompatible with Julia v1.10 and v1.11, cf. CompatHelper: bump compat for JET to 0.10 for package test, (keep existing compat) #194). So I it will probably be hard to let the tests run on both pre-1.12 and 1.12.

Would it make sense to move the JET.jl tests to a new CI job - or multiple of them - to fix this? Alternatively, it seems to be fine to use JET.jl only with a specific version, e.g., the LTS or the newest release.

Would that work? We need to have some compat bound for JET.jl in test/Project.toml. If it is something like v0.9.9 the environment for Julia v1.12 cannot instantiate and if it is something like v0.10, the environment for Julia <v1.12 cannot instantiate even if we do not run any JET.jl tests in either of the tests. If I understand correctly, we would also need different environments for the different jobs or am I missing something?

@JoshuaLampert
Copy link
Member Author

@ranocha, do you have an idea how to fix the world age issue with convergence_test? Even with passing @__MODULE__ it fails.

@ranocha
Copy link
Member

ranocha commented Aug 25, 2025

JET.jl fails to precompile. I think JET.jl is only compatible with Julia v1.12 in newer versions than we currently allow (because newer versions are incompatible with Julia v1.10 and v1.11, cf. CompatHelper: bump compat for JET to 0.10 for package test, (keep existing compat) #194). So I it will probably be hard to let the tests run on both pre-1.12 and 1.12.

Would it make sense to move the JET.jl tests to a new CI job - or multiple of them - to fix this? Alternatively, it seems to be fine to use JET.jl only with a specific version, e.g., the LTS or the newest release.

Would that work? We need to have some compat bound for JET.jl in test/Project.toml. If it is something like v0.9.9 the environment for Julia v1.12 cannot instantiate and if it is something like v0.10, the environment for Julia <v1.12 cannot instantiate even if we do not run any JET.jl tests in either of the tests. If I understand correctly, we would also need different environments for the different jobs or am I missing something?

Yes - we could have a specific setup that installs JET.jl only for one of the Julia versions.

@ranocha
Copy link
Member

ranocha commented Aug 25, 2025

Does it help to use Base.@invokelatest at

l2_error, linf_error = mod.analysis_callback(mod.sol)

(and does it change any performance significantly)?

@JoshuaLampert
Copy link
Member Author

JET.jl fails to precompile. I think JET.jl is only compatible with Julia v1.12 in newer versions than we currently allow (because newer versions are incompatible with Julia v1.10 and v1.11, cf. CompatHelper: bump compat for JET to 0.10 for package test, (keep existing compat) #194). So I it will probably be hard to let the tests run on both pre-1.12 and 1.12.

Would it make sense to move the JET.jl tests to a new CI job - or multiple of them - to fix this? Alternatively, it seems to be fine to use JET.jl only with a specific version, e.g., the LTS or the newest release.

Would that work? We need to have some compat bound for JET.jl in test/Project.toml. If it is something like v0.9.9 the environment for Julia v1.12 cannot instantiate and if it is something like v0.10, the environment for Julia <v1.12 cannot instantiate even if we do not run any JET.jl tests in either of the tests. If I understand correctly, we would also need different environments for the different jobs or am I missing something?

Yes - we could have a specific setup that installs JET.jl only for one of the Julia versions.

Ok, so this would mean we don't have JET.jl in the test/Project.toml, but install it within the action yaml file (e.g., as a new action independent of the current CI.yaml)? Yeah, that would probably work, but is a more complicated setup than I would wish. But if it's the only option we see, I can try to set that up.

@JoshuaLampert
Copy link
Member Author

Does it help to use Base.@invokelatest at

l2_error, linf_error = mod.analysis_callback(mod.sol)

(and does it change any performance significantly)?

Let's try in 14307b4.

@JoshuaLampert
Copy link
Member Author

Does it help to use Base.@invokelatest at

l2_error, linf_error = mod.analysis_callback(mod.sol)

(and does it change any performance significantly)?

Let's try in 14307b4.

l2_error, linf_error = @invokelatest mod.analysis_callback(mod.sol)

gives

ERROR: LoadError: UndefVarError: `sol` not defined in `Main.var"##625"`
The binding may be too new: running in world age 44347, while current world is 44365.

instead of

ERROR: LoadError: UndefVarError: `analysis_callback` not defined in `Main.var"##624"`
The binding may be too new: running in world age 44340, while current world is 44358.

Any idea how to solve this?

@ranocha
Copy link
Member

ranocha commented Aug 26, 2025

Maybe another invokelatest to access mod.sol?

@JoshuaLampert
Copy link
Member Author

JoshuaLampert commented Aug 26, 2025

This fixed the world age issues 👍 This will probably also become relevant for Trixi.jl with Julia v1.12.
Edit: It also doesn't change performance (I compared @benchmark convergence_test(default_example(), [256, 512], interval = 1000) with and without the @invokelatest).

@ranocha
Copy link
Member

ranocha commented Aug 26, 2025

Did the plain convergence_test work as expected before when called from the REPL? Or does it also require the invokelatest fixes?

@JoshuaLampert
Copy link
Member Author

I tested it in Julia v1.11. So it worked. I didn't try from the REPL in Julia v1.12. But I can try.

@JoshuaLampert
Copy link
Member Author

Did the plain convergence_test work as expected before when called from the REPL? Or does it also require the invokelatest fixes?

It fails under Julia v1.12 in the REPL. The invokelatest fixes it.

@coveralls
Copy link
Collaborator

coveralls commented Sep 29, 2025

Pull Request Test Coverage Report for Build 18346023930

Details

  • 16 of 17 (94.12%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 98.551%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/solver.jl 6 7 85.71%
Files with Coverage Reduction New Missed Lines %
src/util.jl 2 95.56%
Totals Coverage Status
Change from base Build 18167088887: 0.3%
Covered Lines: 2312
Relevant Lines: 2346

💛 - Coveralls

Copy link
Member Author

@JoshuaLampert JoshuaLampert left a comment

Choose a reason for hiding this comment

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

CI passes on v1.12 and pre-v1.12 🥳
I would say, we can merge this once v1.12 is released, which shouldn't be in the too far future.
@ranocha, was the separate workflow for JET.jl roughly what you had in mind with

Would it make sense to move the JET.jl tests to a new CI job - or multiple of them - to fix this?

Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me! We need to remember to add the new JET.jl CI job to the list of required tests for PRs.

@JoshuaLampert JoshuaLampert requested a review from ranocha October 8, 2025 14:01
@JoshuaLampert JoshuaLampert marked this pull request as ready for review October 8, 2025 14:01
Copy link
Member

@ranocha ranocha left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants