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

Five arg mul! for UniformScaling and improvement in exp! #40731

Merged
merged 10 commits into from
May 7, 2021

Conversation

jarlebring
Copy link
Contributor

@jarlebring jarlebring commented May 6, 2021

This PR implements a five argument mul! for UniformScaling as third argument and second is Number (and conversely second Number and third UniformScaling).

In particular, it provides the possibility to do in-place add of UniformScaling-objects. See the discussion in https://discourse.julialang.org/t/uniform-scaling-inplace-addition-with-matrix/

The in-place version of A += s*I is

mul!(A,true,s*I,true,true)

As an example / use-case, I have adapted exp! (after #40668) where it was needed four times. Compare:

julia> A=randn(100,100); A=0.001*A/opnorm(A,1);

julia> @btime exp_org!(copy($A));
  740.067 μs (21 allocations: 705.69 KiB)

julia> @btime exp_new!(copy($A));
  702.532 μs (17 allocations: 549.28 KiB)

julia> A=randn(100,100); A=2.3*A/opnorm(A,1);

julia> @btime exp_org!(copy($A));
  1.138 ms (33 allocations: 942.28 KiB)

julia> @btime exp_new!(copy($A));
  1.053 ms (27 allocations: 940.38 KiB)

@jarlebring jarlebring force-pushed the mul_uniformscaling branch 2 times, most recently from 5f1b4e6 to d3f5944 Compare May 6, 2021 11:55
@dkarrasch dkarrasch added the domain:linear algebra Linear algebra label May 6, 2021
@jarlebring jarlebring changed the title Five arg mul! for in-place add of UniformScaling and improvement in exp! Five arg mul! for UniformScaling and improvement in exp! May 6, 2021
@dkarrasch
Copy link
Member

I think we should additionally test the beta = 0 and the alpha = 0 case, and then this is ready to go.

@jarlebring
Copy link
Contributor Author

jarlebring commented May 6, 2021

When we are still at it, I will add mul!(::AbstractMatrix,::UniformScaling,::Number,::Number,::Number).

Copy link
Member

@dkarrasch dkarrasch left a comment

Choose a reason for hiding this comment

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

LGTM.

@dkarrasch dkarrasch added the status:merge me PR is reviewed. Merge when all tests are passing label May 7, 2021
@dkarrasch dkarrasch merged commit 6726ae8 into JuliaLang:master May 7, 2021
@jarlebring jarlebring deleted the mul_uniformscaling branch May 7, 2021 12:27
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
dghosef pushed a commit to dghosef/julia that referenced this pull request May 11, 2021
@simeonschaub simeonschaub removed the status:merge me PR is reviewed. Merge when all tests are passing label May 21, 2021
shirodkara pushed a commit to shirodkara/julia that referenced this pull request Jun 9, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants