wilson-labs / cola

Compositional Linear Algebra
Apache License 2.0
401 stars 26 forks source link

Low Rank dispatch rules #57

Open Fr0do opened 1 year ago

Fr0do commented 1 year ago

Low-rank dispatch rules for Woodbury Identity case of inv and computationally effective trace of low-rank product, resolves #48

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.33% :tada:

Comparison is base (f40c7e1) 78.44% compared to head (0ae5c03) 78.78%.

:exclamation: Current head 0ae5c03 differs from pull request most recent head 190f9f4. Consider uploading reports for the commit 190f9f4 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #57 +/- ## ========================================== + Coverage 78.44% 78.78% +0.33% ========================================== Files 36 36 Lines 2974 2988 +14 ========================================== + Hits 2333 2354 +21 + Misses 641 634 -7 ``` | [Files Changed](https://app.codecov.io/gh/wilson-labs/cola/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wilson-labs) | Coverage Δ | | |---|---|---| | [cola/linalg/diag\_trace.py](https://app.codecov.io/gh/wilson-labs/cola/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wilson-labs#diff-Y29sYS9saW5hbGcvZGlhZ190cmFjZS5weQ==) | `85.71% <100.00%> (+0.78%)` | :arrow_up: | | [cola/linalg/inv.py](https://app.codecov.io/gh/wilson-labs/cola/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wilson-labs#diff-Y29sYS9saW5hbGcvaW52LnB5) | `90.67% <100.00%> (+0.76%)` | :arrow_up: | ... and [19 files with indirect coverage changes](https://app.codecov.io/gh/wilson-labs/cola/pull/57/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=wilson-labs)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mfinzi commented 1 year ago

I didn't realize you're on a fork, I made some edits and here is a diff: diff_output.txt

Though the tests pass (with the adjustment to take into account that fixed_normal_samples -> randn in a recent patch) it seems that the Woodbury dispatch rules are not being hit. This isn't due to any incorrect usage or mistake on your end, but it looks to be a limitation (bug?) in plum-dispatch for parametric types. issubclass(A, Sum[TypeB, TypeC]) will evaluate to true while not matching the dispatch rule for A. I will investigate if this is easily solvable in our cola-plum-dispatch fork that we use.

Fr0do commented 1 year ago

I didn't realize you're on a fork, I made some edits and here is a diff: diff_output.txt

Is there a way to work collaboratively? I can add mainainers of CoLA to my fork as collaborators. Or maybe there is a way I can create a branch in your repo?