Closed BeibinLi closed 5 years ago
Merging #290 into master will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## master #290 +/- ##
=======================================
Coverage 74.27% 74.27%
=======================================
Files 4 4
Lines 622 622
Branches 127 127
=======================================
Hits 462 462
Misses 123 123
Partials 37 37
Impacted Files | Coverage Δ | |
---|---|---|
pyglmnet/pyglmnet.py | 80.44% <100%> (ø) |
:arrow_up: |
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 7f1fbb7...d7c5594. Read the comment docs.
The test on plot_group_lasso.py in CircleCI failed because it exceeds time limit (i.e. 10 minutes). The new group argument makes the training process longer. The GLM._prox(...) function and optimization will take more time than before.
We can solve this problem either by extending the time limit or adding print(...) during the training.
The test on plot_group_lasso.py in CircleCI failed because it exceeds time limit (i.e. 10 minutes). The new group argument makes the training process longer. The GLM._prox(...) function and optimization will take more time than before.
Does it converge / finish on your box though? Perhaps we could change the learning rate to make it converge faster?
The test on plot_group_lasso.py in CircleCI failed because it exceeds time limit (i.e. 10 minutes). The new group argument makes the training process longer. The GLM._prox(...) function and optimization will take more time than before.
Does it converge / finish on your box though? Perhaps we could change the learning rate to make it converge faster?
Yes, it finished in my computer, which takes about one hour. (I did not count the time, but the final plot is nice and should be correct).
@BeibinLi can you update whats_new.rst to document your contribution?
Also, since the plot takes so long to be produced, can you rename the plot_grouplasso.py example and remove the plot prefix so that it is not built during the continuous integration? That way, we can at least catch other failing examples while we work on making this faster.
Merging so we can move on here. Thanks @BeibinLi !