mathurinm / andersoncd

This code is no longer maintained. The codebase has been moved to https://github.com/scikit-learn-contrib/skglm. This repository only serves to reproduce the results of the AISTATS 2021 paper "Anderson acceleration of coordinate descent" by Quentin Bertrand and Mathurin Massias.
BSD 3-Clause "New" or "Revised" License
18 stars 6 forks source link

ENH: Add support for sparse design matrices #54

Closed QB3 closed 2 years ago

QB3 commented 2 years ago

closes #41

codecov-commenter commented 2 years ago

Codecov Report

Merging #54 (a7bc426) into master (b8ce7d3) will increase coverage by 5.75%. The diff coverage is 63.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   56.19%   61.94%   +5.75%     
==========================================
  Files          12       11       -1     
  Lines        1098      657     -441     
  Branches      242      110     -132     
==========================================
- Hits          617      407     -210     
+ Misses        406      217     -189     
+ Partials       75       33      -42     
Impacted Files Coverage Δ
andersoncd/tests/test_docstring_parameters.py 73.91% <ø> (-0.73%) :arrow_down:
andersoncd/datafits.py 40.00% <40.00%> (ø)
andersoncd/penalties.py 43.11% <43.11%> (ø)
andersoncd/solver.py 59.72% <59.72%> (ø)
andersoncd/data/synthetic.py 72.41% <69.23%> (-18.50%) :arrow_down:
andersoncd/__init__.py 100.00% <100.00%> (ø)
andersoncd/data/__init__.py 100.00% <100.00%> (ø)
andersoncd/estimators.py 100.00% <100.00%> (ø)
andersoncd/tests/test_estimators.py 100.00% <100.00%> (ø)
andersoncd/utils.py 28.76% <0.00%> (-4.11%) :arrow_down:
... and 7 more

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 918415f...a7bc426. Read the comment docs.

mathurinm commented 2 years ago

wdyt of having a single kkt_violation, using default arguments, to which you'd alternately pass X or the triplet (data, indices, indptr) as:

def kkt_violation(is_sparse, ..., X=np.zeros(1), data=np.zeros(1), ):
    # stuff 
    if is_sparse:
        # stuff
QB3 commented 2 years ago

wdyt of having a single kkt_violation, using default arguments, to which you'd alternately pass X or the triplet (data, indices, indptr)

I am not sure to understand the benefits of this approach: a function would be removed, but the code quantity would be same no? I tend to like the consistence of the current approach: all jitted functions that take X in argument are duplicated, with the same signature, exept for X being replaced by X.data, X.intptr, X.....

mathurinm commented 2 years ago

For me it reduces code duplication between the two kkt functions

Le mer. 28 juil. 2021 à 11:10, Quentin Bertrand @.***> a écrit :

wdyt of having a single kkt_violation, using default arguments, to which you'd alternately pass X or the triplet (data, indices, indptr)

I am not sure to understand the benefits of this approach: a function would be removed, but the code quantity would be same no? I tend to like the consistence of the current approach: all jitted functions that take X in argument are duplicated, with the same signature, exept for X being replaced by X.data, X.intptr, X.....

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mathurinm/andersoncd/pull/54#issuecomment-888146883, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACETTQTXTR6HNMNRDPSAGSTTZ7CPRANCNFSM5BCJZZ4Q .

mathurinm commented 2 years ago

merge when green @QB3