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

workaround for #40048, stack overflow in type intersection #41091

Closed
wants to merge 1 commit into from

Conversation

JeffBezanson
Copy link
Member

This is yet another case where we end up creating circular variable constraints inside intersection. I have tried some approaches to fixing it, but so far it is proving tricky to both fully avoid the problem and preserve precision.

For some reason this call site has tended to lead to these sorts of tricky intersection cases, and does not seem to be 100% necessary, so I propose disabling it for now.

@JeffBezanson JeffBezanson added the types and dispatch Types, subtyping and method dispatch label Jun 4, 2021
@aviatesk aviatesk added this to the 1.7 milestone Jun 7, 2021
@palday
Copy link
Contributor

palday commented Jun 7, 2021

This seems to fix the problem in MixedModels.jl, but tests will still fail on this because a deprecation in LinearAlgebra means we get the wrong warning in one test. (That will be fixed soon, now that we can actually run tests against 1.7-dev.)

cc: @dmbates

@Sacha0
Copy link
Member

Sacha0 commented Jun 7, 2021

This patch also appears to work around the instance of #40048 that we've been hitting. Thanks Jeff! 🎉

@vtjnash
Copy link
Member

vtjnash commented Jun 9, 2021

This was added to avoid unreliable computations in method-ambiguity detection that was making Base tests fail periodically (it seemed they would fail most often on Windows, as most particularly cursed heisenbugs do). Is type-intersection now accurate enough to avoid those cases?

@JeffBezanson
Copy link
Member Author

I don't know, but the failures in #40048 seem to be a larger problem. Maybe the failures I triggered in #39722 (comment) are a more consistent version of the heisenbug?

@JeffBezanson
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@KristofferC
Copy link
Member

@nanosoldier runtests(["Convex1d", "EarlyStopping", "EcoSISTEM", "FastFloat16s", "GtkObservables", "KLDivergences", "LoggingExtras", "MultiStochGrad", "PLCTag", "Pitaya", "SalesForceBulkApi", "StatsFuns", "Theta", "YAActL"], vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@KristofferC
Copy link
Member

Looks like at least Gtk and Theta are related?

@JeffBezanson
Copy link
Member Author

I could not reproduce them locally. Looks like a general memory corruption kind of issue, but I guess I can't rule out the PR as a cause completely.

@KristofferC
Copy link
Member

I can't repro locally either.

@ararslan
Copy link
Member

@nanosoldier runtests(["Gtk", "Theta"], vs=":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

ProcessExitedException(2)

cc @maleadt

@JeffBezanson JeffBezanson removed this from the 1.7 milestone Jul 23, 2021
@ararslan ararslan deleted the jb/workaround40048 branch August 4, 2021 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants