-
Notifications
You must be signed in to change notification settings - Fork 53
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
Features/343 qr #429
Features/343 qr #429
Conversation
setting loop would sometimes try to set elements which were beyond the shape given simple fix was try except, possible cleaner fix with different logic but this should suffice
outline of qr working. need to implement: - remainder handling in binary reduction step - bug in multiple tile/process code (not working) - ATM this is only split=0
…ters, merging from master
…tation of Q calculation
…ics/heat into features/343-QR
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
=========================================
- Coverage 96.77% 96.6% -0.18%
=========================================
Files 63 65 +2
Lines 12872 13991 +1119
=========================================
+ Hits 12457 13516 +1059
- Misses 415 475 +60
Continue to review full report at Codecov.
|
I ran the tests successful with 1 to 8 GPUs and CPUs. I have also included an extended tests feature for QR. the loops for this equate to the following: st_whole = torch.randn(70, 70, device=device)
for sp in range(0, 1):
for m in range(50, 71, 1):
for n in range(50, 71, 1):
for t in range(1, 3):
st = st_whole[:m, :n].clone()
a_comp = ht.array(st, split=0, device=ht_device)
a = ht.array(st, split=sp, device=ht_device)
qr = a.qr(tiles_per_proc=t)
self.assertTrue(ht.allclose(a_comp, qr.Q @ qr.R, rtol=1e-5, atol=1e-5))
self.assertTrue(
ht.allclose(
qr.Q.T @ qr.Q, ht.eye(m, device=ht_device), rtol=1e-5, atol=1e-5
)
)
self.assertTrue(
ht.allclose(
ht.eye(m, device=ht_device), qr.Q @ qr.Q.T, rtol=1e-5, atol=1e-5
)
) there tests were successfully run on 4,7, and 8 GPUs and CPUs, however they do take about 100x as long. I have successfully run the extended tests for |
Description
Please include a summary of the change and which issue/s is/are fixed.
Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes: #343
Changes proposed
tiles
property to DNDarrayType of change
-> New feature (non-breaking change which adds functionality)
Are all split configurations tested and accounted for?
[x] yes [ ] no
Does this change require a documentation update outside of the changes proposed?
[ ] yes [x] no
Does this change modify the behaviour of other functions?
[ ] yes [x] no
Are there code practices which require justification?
[x] yes [ ] no
-> There are multiple instances of >3 indentations here. Very often this occurs because of rank selection.