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.

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

[WIP] Remove the requirement for double update in forward #612

Closed
wants to merge 20 commits into from

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Nov 9, 2021

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

This PR optimizer the forward method of the Metric base class to prevent performing 2 updates.

Parts of #344

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@tchaton tchaton marked this pull request as ready for review November 9, 2021 16:07
@tchaton tchaton marked this pull request as draft November 9, 2021 16:07
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #612 (dbb480a) into master (c0e4250) will decrease coverage by 25%.
The diff coverage is 38%.

@@           Coverage Diff           @@
##           master   #612     +/-   ##
=======================================
- Coverage      95%    70%    -25%     
=======================================
  Files         166    166             
  Lines        6795   6813     +18     
=======================================
- Hits         6469   4795   -1674     
- Misses        326   2018   +1692     

@tchaton tchaton marked this pull request as ready for review November 9, 2021 18:01
@tchaton tchaton changed the title Remove the requirement for double update in forward [WIP] Remove the requirement for double update in forward Nov 9, 2021
@Borda Borda marked this pull request as draft November 9, 2021 18:30
@tchaton tchaton marked this pull request as ready for review November 9, 2021 18:47
@tchaton tchaton marked this pull request as draft November 9, 2021 18:47
@tchaton tchaton marked this pull request as ready for review November 9, 2021 19:09
@tchaton tchaton marked this pull request as draft November 9, 2021 19:09
@tchaton tchaton marked this pull request as ready for review November 9, 2021 19:15
@Borda Borda added the enhancement New feature or request label Nov 18, 2021
@Borda
Copy link
Member

Borda commented Nov 18, 2021

@tchaton any update here? 🐰

@Borda Borda marked this pull request as draft November 24, 2021 09:24
@Borda
Copy link
Member

Borda commented Dec 17, 2021

@tchaton any update here? 🐰

CHANGELOG.md Outdated Show resolved Hide resolved
@SkafteNicki SkafteNicki added this to the v0.9 milestone Apr 12, 2022
@Borda Borda removed this from the v0.9 milestone Apr 16, 2022
@Borda
Copy link
Member

Borda commented Apr 16, 2022

@SkafteNicki, are we going to proceed with this one? 🐰

@SkafteNicki SkafteNicki mentioned this pull request Apr 25, 2022
4 tasks
@SkafteNicki
Copy link
Member

Closing in favour of #984

@Borda Borda deleted the api_re_design_2 branch May 27, 2022 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request has conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants