Symbolics.jl icon indicating copy to clipboard operation
Symbolics.jl copied to clipboard

noinline in ShardedForm

Open shashi opened this issue 3 years ago • 4 comments

Fixes #651

shashi avatar Jul 23 '22 12:07 shashi

Codecov Report

Merging #664 (0697596) into master (d8373e2) will decrease coverage by 67.84%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #664       +/-   ##
==========================================
- Coverage   76.81%   8.96%   -67.85%     
==========================================
  Files          23      23               
  Lines        2691    2633       -58     
==========================================
- Hits         2067     236     -1831     
- Misses        624    2397     +1773     
Impacted Files Coverage Δ
src/build_function.jl 0.00% <0.00%> (-74.70%) :arrow_down:
src/groebner_basis.jl 0.00% <0.00%> (-100.00%) :arrow_down:
src/semipoly.jl 0.00% <0.00%> (-95.41%) :arrow_down:
src/linear_algebra.jl 0.00% <0.00%> (-89.48%) :arrow_down:
src/array-lib.jl 0.00% <0.00%> (-80.12%) :arrow_down:
src/diff.jl 0.37% <0.00%> (-80.00%) :arrow_down:
src/complex.jl 0.00% <0.00%> (-75.00%) :arrow_down:
src/difference.jl 0.00% <0.00%> (-70.00%) :arrow_down:
src/utils.jl 7.20% <0.00%> (-69.73%) :arrow_down:
... and 12 more

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

codecov-commenter avatar Jul 23 '22 12:07 codecov-commenter

Are there any plans to merge this or is this superseded by the fix in Base? It seems that there is still value in having a fix for older Julia versions (including 1.6 lts)

lassepe avatar Aug 10 '22 21:08 lassepe

We should merge this. I'll have a go in a bit.

shashi avatar Aug 10 '22 22:08 shashi

Sorry to bother you again here but is there any way in which I could assist to push this over the finish line (e.g. by adding a tests for this PR?)

lassepe avatar Sep 14 '22 06:09 lassepe

My code is still suffering from what appears to be this segfault and the fix in julia master does not seem to fully cover that setting. Is there any chance this can branch can be merged?

lassepe avatar Oct 26 '22 11:10 lassepe

Can you share an MWE that we can add as a test?

ChrisRackauckas avatar Oct 31 '22 00:10 ChrisRackauckas

The MWE that triggered the original issue is this. However, after excessive testing in the last few days, learned that the remaining segfaults that I reported above were not the fault of Symbolics.jl but another lib in the same code.

So the fix in Julia master seems to have rendered this PR obsolete as it also has been backported (unless this PR fixes other things beyond #651).

Sorry again for the confusion.

lassepe avatar Nov 14 '22 16:11 lassepe