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

Loosen searchsorted* index type (fixes #30763) #31633

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

kmsquire
Copy link
Member

@kmsquire kmsquire commented Apr 6, 2019

@kmsquire kmsquire force-pushed the kms/fix-search-sorted branch 3 times, most recently from 1179a40 to b0165eb Compare April 6, 2019 19:34
@kmsquire kmsquire changed the title Loosen searchsorted* index type (fixes #30763) WIP: Loosen searchsorted* index type (fixes #30763) Apr 8, 2019
@kmsquire
Copy link
Member Author

kmsquire commented Apr 8, 2019

There were some suggestions left by @StefanKarpinski in 60fc1f9 that were lost because of my force push. Will address those, and loosen the types further to accommodate #31618.

@kmsquire
Copy link
Member Author

kmsquire commented Apr 8, 2019

@StefanKarpinski, one of your recommendation was to change T(1) to one(T). I actually had that at first, but it failed during build with an error that one was not available (i.e., this file is compiled before one is available).

I'm inclined to leave it as T(1), but let me know if you have a different suggestion.

@kmsquire
Copy link
Member Author

kmsquire commented Apr 8, 2019

Tried using one again, just to make sure. The error is here:

error during bootstrap:
LoadError(at "compiler/compiler.jl" line 3: LoadError(at "compiler/bootstrap.jl" line 8: UndefVarError(var=:one)))

I'll change it back to T(1)

Copy link
Member

@StefanKarpinski StefanKarpinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Can be merged whenever CI passes.

@kmsquire kmsquire changed the title WIP: Loosen searchsorted* index type (fixes #30763) Loosen searchsorted* index type (fixes #30763) Apr 8, 2019
@kmsquire
Copy link
Member Author

kmsquire commented Apr 8, 2019

Travis and AppVeyor pass.

Buildbot tests seem unrelated (Cc: @staticfloat):

buildbot/testter_macos64:

Error in testset InteractiveUtils:
Error During Test at /Users/julia/buildbot/worker-tabularasa/tester_macos64/build/share/julia/test/testdefs.jl:20
  Got exception outside of a @test
  Error while loading expression starting at /Users/julia/buildbot/worker-tabularasa/tester_macos64/build/share/julia/stdlib/v1.2/InteractiveUtils/test/runtests.jl:386
  caused by [exception 1]
  failed process: Process(`pbcopy`, ProcessExited(1)) [1]

buildbot/tester_win32:

Error in testset FileWatching:
Error During Test at C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win32\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:238
  Test threw exception
  Expression: ispath(tr[1]) && ispath(tr[2])
  MethodError: no method matching joinpath(::EOFError)
  Closest candidates are:
    joinpath(!Matched::String, !Matched::String) at path.jl:266
    joinpath(!Matched::AbstractString) at path.jl:248
    joinpath(!Matched::AbstractString, !Matched::AbstractString) at path.jl:275

buildbot/tester_win64:

Error in testset spawn:
Test Failed at C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\spawn.jl:366
  Expression: desc == "Pipe($(infd) open => $(outfd) active, 0 bytes waiting)"
   Evaluated: "Pipe(Base.Libc.WindowsRawSocket(0x0000000000000404) open => Base.Libc.WindowsRawSocket(0x00000000000003fc) active, 9 bytes waiting)" == "Pipe(Base.Libc.WindowsRawSocket(0x0000000000000404) open => Base.Libc.WindowsRawSocket(0x00000000000003fc) active, 0 bytes waiting)"
Error in testset FileWatching:
Test Failed at C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:179
  Expression: #= C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:179 =# @elapsed(#= C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:179 =# @test(watch_folder(dir, 0) == ("" => FileWatching.FileEvent()))) <= 2
   Evaluated: 61.374476086 <= 2
Error in testset FileWatching:
Test Failed at C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:208
  Expression: #= C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:208 =# @elapsed(#= C:\cygwin\home\Administrator\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.2\FileWatching\test\runtests.jl:208 =# @test(watch_folder(dir, 0) == ("" => FileWatching.FileEvent()))) <= 0.5
   Evaluated: 1.334171801 <= 0.5

@kmsquire
Copy link
Member Author

kmsquire commented Apr 8, 2019

(Should be squashed on merge)

@StefanKarpinski
Copy link
Member

@staticfloat: I can't tell if these test runs failing is related or not; also, does closing and opening the PR restart these CI runs like it does for other CI services?

@staticfloat
Copy link
Member

Yes, it will. You can also click on the Details link, then click the rebuild button in the top right to re-run them.

  • The MacOS failure is the pbcopy issue, should be fixed on rebase.
  • The Win64 failure has a bunch of FileWatching failures where something that should have happened in <= 2 seconds instead happened in 60 seconds. I don't know what to make of that, but I'm willing to bet it's not this PR's fault.
  • The Win32 failure has some permissions errors; I've noticed this happening every now and then on the windows buildbots, I think it's because a previous Julia run stays resident in memory as a zombie process.

I am restarting the windows builds, but IMO this PR is good to go.

@kmsquire
Copy link
Member Author

Rebased to address the pbcopy issue (#31653) .

The win64 FileWatching also failures appeared again before the rebase, so might look to see if that code calls into this codepath if it appears again.

@kmsquire
Copy link
Member Author

kmsquire commented Apr 12, 2019

The previous errors are gone, but at least one new one has cropped up: a failure while detecting ambiguities in LinearAlgebra on OSX. I did one restart, and the failure is the same.

There was also a new OSX Travis failure that I restarted, but hasn't completed yet. Travis testing passed.

@kmsquire
Copy link
Member Author

kmsquire commented Apr 12, 2019

Repeated LinearAlgebra failure:

      From worker 5:	Error During Test at /Users/sabae/buildbot/worker-tabularasa/tester_macos64/build/share/julia/stdlib/v1.3/LinearAlgebra/test/ambiguous_exec.jl:4
      From worker 5:	  Test threw exception
      From worker 5:	  Expression: detect_ambiguities(LinearAlgebra; imported=true, recursive=true) == []
      From worker 5:	  TypeError: in TypeVar, in upper bound, expected Type, got Core.Compiler.UseRef
      From worker 5:	  Stacktrace:
      From worker 5:	   [1] typeintersect at ./reflection.jl:560 [inlined]
      From worker 5:	   [2] #isambiguous#23(::Bool, ::typeof(Base.isambiguous), ::Method, ::Method) at ./reflection.jl:1266
      From worker 5:	   [3] #isambiguous at ./none:0 [inlined]
      From worker 5:	   [4] #detect_ambiguities#30(::Bool, ::Bool, ::Bool, ::typeof(detect_ambiguities), ::Module) at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1439
      From worker 5:	   [5] (::getfield(Test, Symbol("#kw##detect_ambiguities")))(::NamedTuple{(:imported, :recursive),Tuple{Bool,Bool}}, ::typeof(detect_ambiguities), ::Module) at ./none:0
      From worker 5:	   [6] top-level scope at /Users/sabae/buildbot/worker-tabularasa/tester_macos64/build/share/julia/stdlib/v1.3/LinearAlgebra/test/ambiguous_exec.jl:4
      From worker 5:	   [7] include at ./boot.jl:328 [inlined]
      From worker 5:	   [8] include_relative(::Module, ::String) at ./loading.jl:1094
      From worker 5:	   [9] include(::Module, ::String) at ./Base.jl:31
      From worker 5:	   [10] exec_options(::Base.JLOptions) at ./client.jl:295
      From worker 5:	   [11] _start() at ./client.jl:464
      From worker 5:	  
      From worker 5:	ERROR: Error while loading expression starting at /Users/sabae/buildbot/worker-tabularasa/tester_macos64/build/share/julia/stdlib/v1.3/LinearAlgebra/test/ambiguous_exec.jl:4
      From worker 5:	caused by [exception 1]
      From worker 5:	There was an error during testing
      From worker 5:	running testset LinearAlgebra/bunchkaufman...

@staticfloat, when restarting the buildbot tests, is the patch reapplied to the latest master, or is the branch compiled and run as is?

(In other words, is rebasing on master necessary when errors are fixed. My impression is yes, except maybe on AppVeyor?)

Edit: direct link to failed log: https://build.julialang.org/#/builders/69/builds/632

@staticfloat
Copy link
Member

The buildbots just check out this branch; so yes you need to rebase to incorporate fixes from master.

@kmsquire
Copy link
Member Author

Different error now in buildbot/package_linux32:

error during bootstrap:
LoadError("sysimg.jl", 16, LoadError("/buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.3/LinearAlgebra/src/LinearAlgebra.jl", 355, LoadError("/buildworker/worker/package_linux32/build/usr/share/julia/stdlib/v1.3/LinearAlgebra/src/triangular.jl", 593, TypeError(:TypeVar, "upper bound", Type, Core.Compiler.UseRefIterator((Core.Compiler.UseRef(nothing, 0), nothing), false)))))
rec_backtrace at /buildworker/worker/package_linux32/build/src/stackwalk.c:94
record_backtrace at /buildworker/worker/package_linux32/build/src/task.c:210 [inlined]
jl_throw at /buildworker/worker/package_linux32/build/src/task.c:417
jl_parse_eval_all at /buildworker/worker/package_linux32/build/src/ast.c:895
jl_load at /buildworker/worker/package_linux32/build/src/toplevel.c:861
exec_program at /buildworker/worker/package_linux32/build/ui/repl.c:35
true_main at /buildworker/worker/package_linux32/build/ui/repl.c:108
main at /buildworker/worker/package_linux32/build/ui/repl.c:217
__libc_start_main at /build/glibc-PNN5fi/glibc-2.19/csu/libc-start.c:287
_start at /buildworker/worker/package_linux32/build/usr/bin/julia (unknown line)

Link: https://build.julialang.org/#builders/35/builds/1733

* Might also address JuliaLang#31618
* Types of start and stop indicies are restricted to
  Integer and must be the same type
* Note that this file is compiled early during bootstrap, and `one` is not yet
  available
@kmsquire
Copy link
Member Author

Tests have all passed now. Mergeable?

@kmsquire
Copy link
Member Author

Bump.

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.

3 participants