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

Add polar decompositions #19

Merged
merged 4 commits into from
Nov 16, 2020

Conversation

RalphAS
Copy link
Contributor

@RalphAS RalphAS commented Nov 12, 2020

This change imports updated versions of the polar decomposition routines
originally implemented by Weijian Zhang (https://github.com/weijianzhang/PolarFact.jl).

This change imports updated versions of the polar decomposition routines
originally implemented by Weijian Zhang (https://github.com/weijianzhang/PolarFact.jl).
@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #19 (5f5b906) into master (b55c0de) will increase coverage by 1.18%.
The diff coverage is 94.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   90.00%   91.18%   +1.18%     
==========================================
  Files           5        6       +1     
  Lines         490      669     +179     
==========================================
+ Hits          441      610     +169     
- Misses         49       59      +10     
Impacted Files Coverage Δ
src/MatrixFactorizations.jl 100.00% <ø> (ø)
src/polar.jl 94.41% <94.41%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b55c0de...5f5b906. Read the comment docs.

Copy link
Member

@dlfivefifty dlfivefifty 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! Just some small changes.

In the future we may wish to consider splitting out the iterative solves to another package, e.g., IterativeFactorizations.jl

Project.toml Outdated Show resolved Hide resolved
test/test_polar.jl Outdated Show resolved Hide resolved
@RalphAS RalphAS requested a review from dlfivefifty November 13, 2020 22:29
@dlfivefifty
Copy link
Member

Can you get the coverage up?

Until they can do better than
"an error occurred while generating the build script".
@dlfivefifty
Copy link
Member

Why is OS X allowed to fail?

@RalphAS
Copy link
Contributor Author

RalphAS commented Nov 16, 2020

I suppressed OSX because I encountered infrastructure failures like this one unrelated to the package; the alternative is to ask you to manually restart the checks until they find a lucky instance.

@dlfivefifty dlfivefifty merged commit a15d92e into JuliaLinearAlgebra:master Nov 16, 2020
@RalphAS RalphAS deleted the ras/polarfact branch November 16, 2020 15:34
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.

2 participants