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

Update deprecated &x syntax in blas #23400

Closed
wants to merge 1 commit into from
Closed

Update deprecated &x syntax in blas #23400

wants to merge 1 commit into from

Conversation

musm
Copy link
Contributor

@musm musm commented Aug 22, 2017

part of #6080

@musm musm changed the title Update deprecated in blas Update deprecated &x syntax in blas Aug 22, 2017
@fredrikekre
Copy link
Member

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

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@ararslan
Copy link
Member

Hm, that's weird, let's try again. @nanosoldier runbenchmarks(ALL, vs=":master")

Ref{$relty}, Ref{$elty}, Ref{BlasInt}, Ref{$relty},
Ref{$elty}, Ref{BlasInt}),
uplo, trans, n, k,
α, A, max(1,stride(A,2)), &β,
Copy link
Member

Choose a reason for hiding this comment

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

Missed & here? Nanosoldier logs complained about this so probably the reason for the benchmark failure.

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@musm
Copy link
Contributor Author

musm commented Aug 23, 2017

one more time?

@ararslan
Copy link
Member

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

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@andreasnoack
Copy link
Member

I can confirm that this actually increases the memory usage in the the ["random"]["types"][("rand", "MersenneTwister", "UInt32")] benchmark (but not the number of allocations). @yuyichao Is that expected?

@@ -171,8 +171,8 @@ for (fname, elty) in ((:dcopy_,:Float64),
# SUBROUTINE DCOPY(N,DX,INCX,DY,INCY)
function blascopy!(n::Integer, DX::Union{Ptr{$elty},StridedArray{$elty}}, incx::Integer, DY::Union{Ptr{$elty},StridedArray{$elty}}, incy::Integer)
ccall((@blasfunc($fname), libblas), Void,
(Ptr{BlasInt}, Ptr{$elty}, Ptr{BlasInt}, Ptr{$elty}, Ptr{BlasInt}),
Copy link
Member

Choose a reason for hiding this comment

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

Here and in a few other places, there's no ampersand on the DX and DY. Probably still fine by proper conversions. Just wanted to point it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going off the docs: #23307 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can revert this back if necessary, but it seems that they should in principle be Refs?

@musm musm closed this Aug 23, 2017
@andreasnoack
Copy link
Member

@musm Why close?

@musm
Copy link
Contributor Author

musm commented Aug 24, 2017

@Keno mentioned he has a bot that can fix these, so I thought he could use this and lapack.jl with the bot.

@musm musm deleted the bls branch May 15, 2019 16:59
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.

6 participants