Fix the inference of the function coloring for static compilation - #340
Fix the inference of the function coloring for static compilation#340amontoison wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #340 +/- ##
==========================================
- Coverage 99.20% 97.61% -1.60%
==========================================
Files 21 21
Lines 2151 2219 +68
==========================================
+ Hits 2134 2166 +32
- Misses 17 53 +36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
84cb1b2 to
8625abb
Compare
| # | ||
| # The C and Fortran interfaces compile SMC.jl into a standalone shared library. | ||
| # The trimming verifier rejects any call whose result type is not fully inferred, | ||
| # so `coloring` must return a concrete type. |
There was a problem hiding this comment.
Did these tests use to fail before?
There was a problem hiding this comment.
Yes, they fail on main. It is why I added them.
| problem::ColoringProblem, | ||
| algo::GreedyColoringAlgorithm; | ||
| decompression_eltype::Type{R}=Float64, | ||
| decompression_eltype::Type=Float64, |
There was a problem hiding this comment.
The rationale behind the type annotation was to force specialization: https://docs.julialang.org/en/v1/manual/performance-tips/#Be-aware-of-when-Julia-avoids-specializing
Not sure how this change makes anything better in that regard, I would have thought it makes things worse?
There was a problem hiding this comment.
Me too and it is the contrary that is happening, the compiler widen the type to DataType internally from what I unserstand if we keep Type{R} and we don't have the inference in 3 cases.
I am wondering if it is because the argument with the specialization is a keyword argument.
@gdalle I will open a follow-up PR with C and Fortran interfaces but we first need to fix the inference of the function
coloring.