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

noinline in ShardedForm #664

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

noinline in ShardedForm #664

wants to merge 2 commits into from

Conversation

shashi
Copy link
Member

@shashi shashi commented Jul 23, 2022

Fixes #651

@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2022

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%) ⬇️
src/groebner_basis.jl 0.00% <0.00%> (-100.00%) ⬇️
src/semipoly.jl 0.00% <0.00%> (-95.41%) ⬇️
src/linear_algebra.jl 0.00% <0.00%> (-89.48%) ⬇️
src/array-lib.jl 0.00% <0.00%> (-80.12%) ⬇️
src/diff.jl 0.37% <0.00%> (-80.00%) ⬇️
src/complex.jl 0.00% <0.00%> (-75.00%) ⬇️
src/difference.jl 0.00% <0.00%> (-70.00%) ⬇️
src/utils.jl 7.20% <0.00%> (-69.73%) ⬇️
... and 12 more

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

@lassepe
Copy link

lassepe commented Aug 10, 2022

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)

@shashi
Copy link
Member Author

shashi commented Aug 10, 2022

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

@lassepe
Copy link

lassepe commented Sep 14, 2022

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
Copy link

lassepe commented Oct 26, 2022

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?

@ChrisRackauckas
Copy link
Member

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

@lassepe
Copy link

lassepe commented Nov 14, 2022

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.

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.

Crashes in calls of functions generated with Symbolics.build_function
4 participants