Skip to content

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 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 merge me PR is reviewed. Merge when all tests are passing label May 7, 2021
@jarlebring jarlebring force-pushed the mul_uniformscaling branch from ed461bb to 78fb45b Compare May 7, 2021 07:49
@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 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

linear algebra Linear algebra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants