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

Make copyto!(A,I) work for rectangular matrices #28790

Merged
merged 3 commits into from Aug 22, 2018

Conversation

4 participants
@Jutho
Copy link
Contributor

commented Aug 21, 2018

Given that Matrix{T}(I, (m,n)) works for rectangular matrices m != n, I would also expect copyto!(A,I) to work. I often use this to initialize an existing matrix with ones on the diagonal, even if it is rectangular.

An example is in LinearAlgebra, suppose you want to materialize the q factor in a QR decomposition. You might have two equally sized matrices A and B lying around:

using LinearAlgebra
Q,R = qr!(A)
Qmaterialized = lmul!(Q, copyto!(B,I))

I'll probably need to update some tests as well?

@Sacha0

This comment has been minimized.

Copy link
Member

commented Aug 21, 2018

I'll probably need to update some tests as well?

Tests would be lovely! It seems the UniformScaling tests lack anything for copyto! presently? Best!

Jutho added some commits Aug 22, 2018

add tests for `copyto!`
add tests for `copyto!(::Matrix, ::UniformScaling)`
@Sacha0

Sacha0 approved these changes Aug 22, 2018

Copy link
Member

left a comment

Thanks @Jutho! :)

@Jutho

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

You're welcome. I assume the one travis error is unrelated? Sorry about the whitespace error at first; I edited online using Github, and indenting a blank line the same as the surrounding code lines seems to be the default.

@mbauman

This comment has been minimized.

Copy link
Member

commented Aug 22, 2018

Hah, funny, yes, that test failure is unrelated. Looks like it's the totally_not_five26034 test struct that introduces ambiguities… and it'll only fail if that same worker ends up doing the linalg tests.

@mbauman mbauman merged commit 6a2dcc6 into JuliaLang:master Aug 22, 2018

4 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
ci/circleci: build-i686 Your tests passed on CircleCI!
Details
ci/circleci: build-x86_64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
julia freebsd ci Build done
Details

@Jutho Jutho deleted the Jutho:patch-2 branch Aug 23, 2018

staticfloat added a commit that referenced this pull request Aug 24, 2018

Make copyto!(A,I) work for rectangular matrices (#28790)
* Make copyto!(A,I) work for rectangular matrices

* add tests for `copyto!`

add tests for `copyto!(::Matrix, ::UniformScaling)`

* fix whitespace

mortenpi added a commit to mortenpi/julia that referenced this pull request Dec 1, 2018

News and compat for JuliaLang#28790
Add a docstring for copyto!(::AbstractMatrix, ::UniformScaling) and
document 1.1 changes.

mortenpi added a commit to mortenpi/julia that referenced this pull request Dec 1, 2018

News and compat annotation for JuliaLang#28790
Add a docstring for copyto!(::AbstractMatrix, ::UniformScaling) and
document 1.1 changes.

mortenpi added a commit to mortenpi/julia that referenced this pull request Dec 1, 2018

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Addition of NEWS and compat admonitions for important
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Addition of NEWS and compat admonitions for important
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Compat admonitions and NEWS for Julia 1.1 (#30230)
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.