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

resolve method ambiguity with SciMLOperators #1766

Merged
merged 7 commits into from
Sep 22, 2022

Conversation

vpuri3
Copy link
Member

@vpuri3 vpuri3 commented Sep 21, 2022

this is to catch a method ambiguity arising in SciML/SciMLBase.jl#210.

Eventual plan is to define WOperator with SciMLOperators tooling but this quickfix should be good for now

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

getting unrelated error (locally)

Ambiguity Tests: Test Failed at /Users/vp/.julia/dev/OrdinaryDiffEq/test/interface/ambi
guity_tests.jl:2                                                                       
  Expression: isempty(Test.detect_ambiguities(OrdinaryDiffEq))                         
   Evaluated: isempty(Tuple{Method, Method}[(\(W::OrdinaryDiffEq.StaticWOperator, v) in
 OrdinaryDiffEq at /Users/vp/.julia/dev/OrdinaryDiffEq/src/derivative_utils.jl:19, \(d1
, d2::IntervalSets.Domain) in DomainSets at /Users/vp/.julia/packages/DomainSets/lfFfE/
src/generic/setoperations.jl:305)])                                                    ────────────────59]]]]]]]]]]]]]]]]]]]]]3]9]]]]]]]1641]37]35]]0]]]]]21]0]]]]]                      ueryDas5ypeof(O_PRECS)rd},thing,fEq.REC//10fals Val{orardyDf.r(u,rnyEqas0fs tn pfrnyfq
Stacktrace:                                                                            
 [1] top-level scope                                                                   
   @ ~/.julia/juliaup/julia-1.8.1+0.aarch64/share/julia/stdlib/v1.8/Test/src/Test.jl:46
4                                                                                      
 [2] include(mod::Module, _path::String)                                               
   @ Base ./Base.jl:419                                                                
 [3] include(x::String)                                                                
   @ Main.var"##348" ~/.julia/packages/SafeTestsets/A83XK/src/SafeTestsets.jl:23       
 [4] macro expansion                                                                   
   @ ~/.julia/dev/OrdinaryDiffEq/test/runtests.jl:63 [inlined]                         
 [5] macro expansion                                                                   
   @ ~/.julia/juliaup/julia-1.8.1+0.aarch64/share/julia/stdlib/v1.8/Test/src/Test.jl:13
57 [inlined]                                                                           
 [6] top-level scope                                                                   
   @ ~/.julia/dev/OrdinaryDiffEq/test/runtests.jl:63                                   
Test Summary:   | Fail  Total  Time                                                    
Ambiguity Tests |    1      1  1.4s                                                    
ERROR: LoadError: Some tests did not pass: 0 passed, 1 failed, 0 errored, 0 broken.    
in expression starting at /Users/vp/.julia/dev/OrdinaryDiffEq/test/runtests.jl:16      
ERROR: Package OrdinaryDiffEq errored during testing                                   ```

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

When run with SciML/SciMLBase.jl#210 branch

Ambiguity Tests: Test Failed at /Users/vp/.julia/dev/OrdinaryDiffEq/test/interface/ambiguity_tests.jl:2                       
  Expression: isempty(Test.detect_ambiguities(OrdinaryDiffEq)) 
   Evaluated: isempty(Tuple{Method, Method}[(*(W::OrdinaryDiffEq.WOperator, x::Union{Number, AbstractVecOrMat}) in OrdinaryDif
fEq at /Users/vp/.julia/dev/OrdinaryDiffEq/src/derivative_utils.jl:374, *(L::SciMLOperators.AbstractSciMLOperator, λ::Number) 
in SciMLOperators at /Users/vp/.julia/packages/SciMLOperators/045lQ/src/basic.jl:203)])                                       ```

@vpuri3 vpuri3 closed this Sep 21, 2022
@vpuri3 vpuri3 reopened this Sep 21, 2022
@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

@ChrisRackauckas any way these tests can be sped up. Waiting in queue for the past hour now ://

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

tests passing locally except for the iffy norecompile test

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

@ChrisRackauckas not sure why workflows aren't running, buildkite ran with just one error (in /test/gpu/autoswitch.jl which also errors on master for me so is unrelated to this PR). So this PR should be good to go

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 21, 2022

@vpuri3 vpuri3 closed this Sep 21, 2022
@vpuri3 vpuri3 reopened this Sep 21, 2022
@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

@ChrisRackauckas this is good. the failing check is a timeout in loading delaydiffeq

@vpuri3 vpuri3 closed this Sep 22, 2022
@vpuri3 vpuri3 reopened this Sep 22, 2022
@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

rerunning CI. hopefully delaydiffeq workflow will load

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

the delaydiffeq test is just timing out:

Run julia-actions/setup-julia@v1
Error: connect ETIMEDOUT 146.75.30.49:443

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

@ChrisRackauckas can we just merge?

vpuri3 added a commit to vpuri3/OrdinaryDiffEq.jl that referenced this pull request Sep 22, 2022
@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

ok the delaydiffeq test ran and passed.

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

@ChrisRackauckas can we merge? The multithreading CI has passed before. no idea why it's been loading for the past 4 hours

@vpuri3
Copy link
Member Author

vpuri3 commented Sep 22, 2022

ok the multithreading CI also passed lets goo

@ChrisRackauckas ChrisRackauckas merged commit 68c28bb into SciML:master Sep 22, 2022
@vpuri3 vpuri3 deleted the ambig branch September 22, 2022 23:16
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.

2 participants