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

Feature/338 matrix inverse #875

Merged
merged 20 commits into from
Jan 31, 2022
Merged

Feature/338 matrix inverse #875

merged 20 commits into from
Jan 31, 2022

Conversation

mtar
Copy link
Collaborator

@mtar mtar commented Oct 1, 2021

Description

An implementation of the matrix inverse using Gauss-Jordan elimination when the matrix is distributed.
torch.linalg.inv is called on the local tensors otherwise.

Issue/s resolved: #338

Changes proposed:

  • add linalg.inv

Type of change

  • New feature

Due Diligence

  • All split configurations tested
  • Multiple dtypes tested in relevant functions
  • Documentation updated (if needed)
  • Updated changelog.md under the title "Pending Additions"

Does this change modify the behaviour of other functions? If so, which?

no

@mtar mtar added the API Anything relating the API label Oct 1, 2021
@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #875 (a17d126) into master (dbb8300) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #875      +/-   ##
==========================================
+ Coverage   95.47%   95.49%   +0.02%     
==========================================
  Files          64       64              
  Lines        9741     9793      +52     
==========================================
+ Hits         9300     9352      +52     
  Misses        441      441              
Flag Coverage Δ
gpu 94.58% <100.00%> (+0.02%) ⬆️
unit 91.07% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
heat/core/linalg/basics.py 95.52% <100.00%> (+0.26%) ⬆️

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 dbb8300...a17d126. Read the comment docs.

@mtar mtar changed the base branch from master to bug/866-op-nested October 1, 2021 12:03
Base automatically changed from bug/866-op-nested to master October 8, 2021 09:55
@mtar mtar marked this pull request as ready for review October 11, 2021 13:43
@ClaudiaComito ClaudiaComito added this to the 1.2.x milestone Oct 27, 2021
@coquelin77
Copy link
Member

so ive done a bit of testing on this branch.

ht.random.seed(42)
split = 1
a = ht.random.random((20, 20), dtype=ht.float64, split=split)
ainv = ht.linalg.inv(a)
i = ht.eye(a.shape, split=split, dtype=a.dtype)
print(ht.max((a @ ainv) - i))
print(ht.allclose(a @ ainv, i))

The above code give this:

(base) daniel@daniel-LIFEBOOK-U749:~/.git/heat$ mpirun -np 1 python testing.py 
DNDarray([1.5736e-13], dtype=ht.float64, device=cpu:0, split=None)
True
(base) daniel@daniel-LIFEBOOK-U749:~/.git/heat$ mpirun -np 2 python testing.py 
DNDarray([3.5616e-13], dtype=ht.float64, device=cpu:0, split=None)
True

However, if i use split=0...

(base) daniel@daniel-LIFEBOOK-U749:~/.git/heat$ mpirun -np 1 python testing.py 
DNDarray([1.2335e-05], dtype=ht.float64, device=cpu:0, split=None)
False
(base) daniel@daniel-LIFEBOOK-U749:~/.git/heat$ mpirun -np 2 python testing.py 
DNDarray([1.2335e-05], dtype=ht.float64, device=cpu:0, split=None)
False

Is this some artifact of the algorithm?

@mtar
Copy link
Collaborator Author

mtar commented Jan 28, 2022

@coquelin77 This is some weird behaviour of the indexing methods of DNDarray and the division operation . It's now the same precision on both splits.

@coquelin77 coquelin77 merged commit c6ebf00 into master Jan 31, 2022
@coquelin77 coquelin77 deleted the feature/338-matrix-inverse branch January 31, 2022 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Anything relating the API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement matrix inverse
3 participants