Skip to content
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.

Sign up for GitHub

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

Jump to bottom

docs/progress bar #347

Merged
merged 10 commits into from
Apr 6, 2023
Merged

docs/progress bar #347

merged 10 commits into from
Apr 6, 2023

Conversation

bosr
Copy link
Contributor

@bosr bosr commented Mar 30, 2023

  1. Slight update ott.utils.default_progress_fn

    I chose to increment by 1 the value of iteration in the function body because ott starts counting from 0 but yields progress updates such as 6/199. I think it's better to report progress such as 7/200.

  2. Add example in the Getting Started notebook

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@michalk8
Copy link
Collaborator

RTD fails because of: pydata/pydata-sphinx-theme#1274

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023 •
edited
Loading

Codecov Report

Merging #347 (6e82e7a) into main (1feb317) will decrease coverage by 0.08%.
The diff coverage is 74.13%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #347      +/-   ##
==========================================
- Coverage   88.30%   88.22%   -0.08%     
==========================================
  Files          51       51              
  Lines        5464     5488      +24     
  Branches      824      828       +4     
==========================================
+ Hits         4825     4842      +17     
- Misses        522      527       +5     
- Partials      117      119       +2
Impacted Files Coverage Δ
src/ott/initializers/linear/initializers_lr.py 92.30% <ø> (ø)
src/ott/utils.py 66.66% <0.00%> (ø)
src/ott/problems/quadratic/quadratic_problem.py 86.45% <69.44%> (-2.25%) ⬇️
src/ott/solvers/quadratic/gromov_wasserstein.py 88.72% <75.00%> (-0.60%) ⬇️
src/ott/geometry/geometry.py 92.66% <100.00%> (ø)
src/ott/initializers/quadratic/initializers.py 86.88% <100.00%> (ø)
src/ott/solvers/linear/sinkhorn.py 96.83% <100.00%> (ø)
src/ott/solvers/linear/sinkhorn_lr.py 96.28% <100.00%> (+0.11%) ⬆️

@michalk8 michalk8 self-requested a review March 30, 2023 17:43
@michalk8 michalk8 added the enhancement New feature or request label Mar 30, 2023
marcocuturi
marcocuturi reviewed Mar 30, 2023
View reviewed changes
Copy link
Contributor

@marcocuturi marcocuturi left a comment

Choose a reason for hiding this comment

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

Thanks!!! LGTM on my end, pending small modifications (which I agree with) highlighted by michal on links to existing doc.

src/ott/utils.py Show resolved Hide resolved
@bosr
Copy link
Contributor Author

bosr commented Mar 31, 2023

Thanks for the feedback, I'll update the code hopefully soon.

@michalk8
Copy link
Collaborator

Thanks for the feedback, I'll update the code hopefully soon.

No problem!

RTD fails because of: pydata/pydata-sphinx-theme#1274

In #348, I also bumped the version of sphinx-book-theme>=1.0.1 since the issue there is fixed.

@bosr Review Notebook App
Copy link
Contributor Author

bosr commented Apr 3, 2023

Thanks @michalk8!

@michalk8
Copy link
Collaborator

michalk8 commented Apr 6, 2023

Thanks @bosr , looks great!

@michalk8 michalk8 merged commit 435c101 into ott-jax:main Apr 6, 2023
@bosr bosr deleted the docs/progress-bar branch April 6, 2023 12:17
michalk8 added a commit that referenced this pull request Jun 27, 2024
@bosr @michalk8
docs/progress bar (#347)
c516533 
* more sensible progress_fn with iteration + 1

* add progress_fn example in Getting Started

* Pin `pydata` theme

* create a new tracking progress notebook

* [ci skip] Link notebooks

* address review comments

* fix typo

* expose LRSinkhornState in docs/solvers/linear.rst

* address review comments

---------

Co-authored-by: rbossart <[email protected]>
Co-authored-by: Michal Klein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalk8 michalk8 michalk8 left review comments

@marcocuturi marcocuturi marcocuturi left review comments

Assignees

@bosr bosr

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bosr @michalk8 @codecov-commenter @marcocuturi