Skip to content

Comments

Use example scripts as tests.#277

Merged
sfarrens merged 17 commits intoCEA-COSMIC:developfrom
paquiteau:example2test
Mar 15, 2023
Merged

Use example scripts as tests.#277
sfarrens merged 17 commits intoCEA-COSMIC:developfrom
paquiteau:example2test

Conversation

@paquiteau
Copy link
Contributor

Hi there,
This PR is a proposal to tackle the current main weaknesses of Modopt:

  1. The use of dummy test to validate coverage (especially for optimization algorithms)
  2. Its lack of examples

The idea is too use examples/tutorials script as "real-world" test cases (or at least more real than checking if the FB algorithm is able to solve the inverse problem y = x with x positive) .

In essence the same file is usable by pytest to solve (1.) and sphinx-gallery to address (2.)

For now I have implemented the LASSO problem (from the simulation example of the original article 1. Most of the algorithms paper have such toy examples, and reproducing them in the example is a good way of showing what ModOpt is capable, and help in the discoverability of its API (the lasso example produces 41% of coverage all by itself !).

A few remarks:

  • Note that I am not talking about benchmarking, I think this should be addressed separatly with e.g. benchop
  • This is still a proof of concept, and some ideas are still on the table:
    • Separating the example script into several code and test them sequentially
    • Test multiple algorithm in the same file, and produce a rough comparison on this toy example (e.g. all the flavour of the FB algorithm).
  • I have not yet configured the sphinx-gallery stuff, stay tuned.

I will greatly appreciate your feedback on this to see if we should continue in this direction.

Footnotes

  1. Tibshirani, 1996 (https://www.jstor.org/stable/2346178)

@paquiteau paquiteau requested review from philouc and sfarrens January 14, 2023 17:29
@paquiteau
Copy link
Contributor Author

A preview of the rendered example can be found here

https://perso.crans.org/comby/neurospin/modopt/example/auto_examples/index.html

Copy link
Contributor

@sfarrens sfarrens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow @paquiteau this is really fantastic and 100% in the spirit of what I hoped for ModOpt when I started the package. 👏

I opened some threads to just fix some typos and tweak the docstrings, but they are all suggestions you can click to commit.

I really like the direction this is going and I think this will hugely increase the understanding and appreciation of all the tools that are already in ModOpt. Some minor comments/suggestions:

  • Even if this a toy example, it helps to visualise the data and reconstruction to demonstrate that things are working. So perhaps a final plot of BETA_TRUE compared to fb_fista.x_final.
  • Another nice feature is to link any mention of specific modules/operators etc. to the corresponding API page. You can do this by writing e.g.:
    """
    :py:func:`Linear <modopt.opt.linear>`
    """
    in the doctstrings (markdown cell lines). This way users can easily find more information about the tools being demonstrated.
  • Even if everything is well referenced to specific examples in papers etc., it might be worth adding a tiny bit of LaTeX maths to some cells to give the big picture of the problem being solved.

Anyway, I am completely happy for you to play around with this format and I think we can quickly converge to a standardised format for these examples.

paquiteau and others added 9 commits January 19, 2023 18:05
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
@paquiteau
Copy link
Contributor Author

Alright I expanded the example a little, here is a new render:
https://perso.crans.org/comby/neurospin/modopt/example/auto_examples/index.html

@sfarrens
Copy link
Contributor

Hi @paquiteau, very sorry for the slow review. 🙏

Can you rebase/pull the latest changes to the develop branch so that I double check that everything looks OK in this PR before merging?

@paquiteau
Copy link
Contributor Author

Hello, everything should be good to go :)

@paquiteau
Copy link
Contributor Author

Ah! I merged a bit to quickly and removed the dependencies to matplotlib for the test ...

@sfarrens sfarrens merged commit 2d7da27 into CEA-COSMIC:develop Mar 15, 2023
chaithyagr added a commit that referenced this pull request Jan 19, 2024
* Add support for tensorflow backend which allows for differentiability (#112)

* Added support for tensorflow

* Updates to get tests passing

* Or --> And

* Moving modopt to allow working with tensorflow

* Fix issues with wos

* Fix all flakes finally!

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Minute updates to codes

* Add dynamic module

* Fix docu

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Fix 115 (#116)

* Fix issues

* Add right tests

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Minor bug fix, remove elif (#124)

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Add tests for modopt.base.backend and fix minute bug uncovered (#126)

* Minor bug fix, remove elif

* Add tests for backend

* Fix tests

* Add tests

* Remove cupy

* PEP fixes

* Fix PEP

* Fix PEP and update

* Final PEP

* Update setup.cfg

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update test_base.py

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Release cleanup (#128)

* updated GPU dependencies

* added logo to manifest

* updated package version and release date

* Unpin package dependencies (#189)

* unpinned dependencies

* updated pinned documentation dependency versions

* Add Gradient descent algorithms (#196)

* Version 1.5.1 patch release (#114)

* Add support for tensorflow backend which allows for differentiability (#112)

* Added support for tensorflow

* Updates to get tests passing

* Or --> And

* Moving modopt to allow working with tensorflow

* Fix issues with wos

* Fix all flakes finally!

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/base/backend.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Minute updates to codes

* Add dynamic module

* Fix docu

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Fix 115 (#116)

* Fix issues

* Add right tests

* Fix PEP

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Minor bug fix, remove elif (#124)

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Add tests for modopt.base.backend and fix minute bug uncovered (#126)

* Minor bug fix, remove elif

* Add tests for backend

* Fix tests

* Add tests

* Remove cupy

* PEP fixes

* Fix PEP

* Fix PEP and update

* Final PEP

* Update setup.cfg

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update test_base.py

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Release cleanup (#128)

* updated GPU dependencies

* added logo to manifest

* updated package version and release date

Co-authored-by: Chaithya G R <chaithyagr@gmail.com>
Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* make algorithms a module.

* add Gradient Descent Algorithms

* enforce WPS compliance.

* add test for gradient descent

* Docstrings improvements

* Add See Also and minor corrections

* add idx initialisation for all algorithms.

* fix merge error

* fix typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Chaithya G R <chaithyagr@gmail.com>
Co-authored-by: chaithyagr <chaithyagr@gitlab.com>

* Release cleanup (#198)

* started clean up for next release

* update progress

* further clean up

* additional clean up

* cleaned up link to logo

* fixed index.rst

* fixed conflict

* Fast Singular Value Thresholding  (#209)

* add SingularValueThreshold

This Method provides 10x faster SVT estimation than the LowRankMatrix Operator.

* linting

* add test for fast computation.

* flake8 compliance

* Ignore DAR000 Error.

* Update modopt/signal/svd.py

tuples in docstring

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/signal/svd.py

typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/opt/proximity.py

typo

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* update docstring

* fix isort

* Update modopt/signal/svd.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/signal/svd.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* run isort

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* added writeable input data array feature for benchopt (#213)

* removed flake8 limit

* updated patch version

* [lint] pydocstyle compliance. (#228)

* [lint] pydocstyle compliance.

* use pytest-pydocstyle

* Power method: fix #211 (#212)

* Correct the norm update for Power Method

x_new should be divided by its norm, not by x_old_norm.

* fix test value

We are testing for eigen value of Identity. It should be one.

* fix WPS350

* fix test value for unconverged case

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Switch from progressbar to tqdm (#231)

* switch from progressbar to tqdm.

The progress bar can be provided externally for nested usage.

* exposes the progress bar argument.

* Child classes better have to implement these.

(my linter was complaining)

* update docs for progress bar using tqdm.

* fix WPS errors

* drop progressbar requirement, add tqdm.

* [lint] disable warning for non implemented function.

* simplify progbar check and argument passthrough

* Update README for tqdm dependency (#240)

Remote progressbar, use tqdm.

* add small help for the metric argument. (#241)

* add small help for the metric argument.

* RST validation

* use single quote

* use double backticks.

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* add implementation for admm and fast admm.

Based on Goldstein2014

* add Goldstein ref.

* WPS compliance.

* Abstract class for cost function.

* add custom cost operator for admm.

* fix WPS compliance.

* Ci update (#268)

* update python version support.

* use string for CI.

* remove flake8 and wemake-python-styleguide

This anticipates the change to black formatting.

* remove wps checks

* apparently conda does not support 3.11 for now

* remove all linting testing.

* fix np.int warning/error

* fix dtype error

* fix precision for doctest

* added black and isort support

* Update python version in README

* add 3.7 for test  back

* don't test 3.10 twice

* Test rewrite (#266)

* add MatrixOperator.

* move base test to pytest.

* [fixme] remove flake8 and emoji config.

* rewrite test_math module using pytest.

* use fail/skipparam helper function.

* generalize usage of failparam

* refactor test_signal.

* refactor test_signal, the end.

* lint

* fix missing parameter.

* add dummy object test helper.

* rewrite test for cost and gradients.

* show missing lines in coverage reports

* rewrite of proximity operators testing.

* add fail  low rank method.

* add cases for algorithms test

* add algorithm test.

* add pytest-cases and pytest-xdists support.

* add support for testing metrics.

* improve base module coverage.

* test for wrong mask in metric module.

* add docstring.

* update email adress and authors field.

* 100% coverage for transform module.

* move linear operator to class

* update docstring.

* paramet(e)rization.

* update docstring.

* improve test_helper module.

* raises should be specified for each failparam call.

* encapsulate module's test in classes.

* skip test if sklearn is not installed.

* pin pydocstyle

* removed unnormalised Gaussian kernel option and corresponding test

* Restrict scikit-image version for testing

* added fix for basic test suite

* set behaviour for different astropy versions

* updated docstring for gaussian_kernel

* Use example scripts as  tests. (#277)

* Initialize the example module.

* do not export the assert statements.

* add matplotlib as requirement.

* add support for sphinx-gallery

* Update modopt/examples/README.rst

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/__init__.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/conftest.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/example_lasso_forward_backward.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* Update modopt/examples/example_lasso_forward_backward.py

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* ignore auto_example folder

* doc formatting.

* add pogm and basic comparison.

* fix: add matplotlib for the plotting in examples scripts.

* fix: add matplotlib for basic ci too.

* ci: run pytest with xdist for faster testing

---------

Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>

* fix: specify data_range for ssim.

Refs: #290

* typos.

* feat(test): add test for admm.

* feat(admm): improve doc.

* refactor: rename abstract cost to CostParent.

* feat: add test for fast admm.

* feat(admm): improve docstrings.

* style: remove extra line.c

* feat: make POGM more memory efficient.

* feat: add a dummy cost for the identity operator.

* feat: create a linear operator module, add wavelet transform.

* feat: add test case for wavelet transform.

* Update setup.py

---------

Co-authored-by: chaithyagr <chaithyagr@gitlab.com>
Co-authored-by: Samuel Farrens <samuel.farrens@gmail.com>
Co-authored-by: Pierre-Antoine Comby <77174042+paquiteau@users.noreply.github.com>
Co-authored-by: Pierre-antoine Comby <pierre-antoine.comby@crans.org>
Co-authored-by: Pierre-Antoine Comby <pierre-antoine.comby@ens-paris-saclay.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants