8000 Add unit tests for image utils unet functions by vijayi1 · Pull Request #3921 · ludwig-ai/ludwig · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add unit tests for image utils unet functions #3921

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
Merged
merged 3 commits into from
Mar 4, 2024
Merged

Add unit tests for image utils unet functions #3921

merged 3 commits into from
Mar 4, 2024

Conversation

vijayi1
Copy link
Contributor
@vijayi1 vijayi1 commented Jan 30, 2024

This is a follow up to PR 3913.
It adds unit tests for the image utils functions added by that PR.

@github-actions GitHub Actions
Copy link
github-actions bot commented Jan 31, 2024 •
edited
Loading

Unit Test Results

  4 files  ±0    4 suites  ±0   9m 14s ⏱️ - 2m 55s
12 tests ±0    7 ✔️  - 2    5 💤 +2  0 ❌ ±0 
40 runs  ±0  20 ✔️  - 8  20 💤 +8  0 ❌ ±0 

Results for commit 340eb69. ± Comparison against base commit 6723a30.

This pull request skips 2 tests. 
tests.regression_tests.benchmark.test_model_performance ‑ test_performance[ames_housing.gbm.yaml]
tests.regression_tests.benchmark.test_model_performance ‑ test_performance[mercedes_benz_greener.gbm.yaml]

♻️ This comment has been updated with latest results.

justinxzhao
justinxzhao approved these changes Feb 1, 2024
View reviewed changes
Copy link
Contributor
@justinxzhao justinxzhao left a comment

Choose a reason for hiding this comment

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

Thank you!

@justinxzhao
Copy link
Contributor

The regression tests are flaky, however it looks like there's another dependency issue going on with torch nightly.

arnavgarg1
arnavgarg1 approved these changes Feb 4, 2024
View reviewed changes
Copy link
Contributor
@arnavgarg1 arnavgarg1 left a comment

Choose a reason for hiding this comment

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

LGTM! Can you rebase with master and see if the tests pass?

@vijayi1
Copy link
Contributor Author
vijayi1 commented Feb 5, 2024

@arnavgarg1 sure. can you please confirm if the following steps are correct to rebase a PR and make additional changes (if needed),

To rebase my branch:
git fetch upstream
git rebase upstream/master

To make further code changes:
git add .
git commit
git push -u origin unet-image-utils-tests

@arnavgarg1
Copy link
Contributor

@vijayi1 I think that mostly looks correct! I would change the ordering of the steps to be this instead:

  1. git fetch upstream
  2. git checkout unet-image-utils-tests (you want to make sure you're on your branch)
  3. git rebase upstream/master (rebase onto your branch)

Followed by:
git add .
git commit -m
git push -u origin unet-image-utils-tests

@vijayi1
Copy link
Contributor Author
vijayi1 commented Feb 6, 2024

@arnavgarg1, I followed those steps exactly and got the following error during push,

(dl_venv) [vijayi@sparky01 ludwig]$ git push -u origin unet-image-utils-tests
To github.com:vijayi1/ludwig.git
! [rejected] unet-image-utils-tests -> unet-image-utils-tests (non-fast-forward)
error: failed to push some refs to 'github.com:vijayi1/ludwig.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
(dl_venv) [vijayi@sparky01 ludwig]$

@arnavgarg1
Copy link
Contributor

@vijayi1 I see. Okay, if you abort the rebase, another option is to just pull the latest master and merge it into your branch - it should achieve roughly the same effect?

@vijayi1
Copy link
Contributor Author
vijayi1 commented Feb 6, 2024

@arnavgarg1, merge completed and I added a few description changes for the docs. the utils tests passed.

@arnavgarg1 arnavgarg1 merged commit 7afc6dc into ludwig-ai:master Mar 4, 2024
@vijayi1 vijayi1 deleted the unet-image-utils-tests branch March 19, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinxzhao justinxzhao justinxzhao approved these changes

@arnavgarg1 arnavgarg1 arnavgarg1 approved these changes

@w4nderlust w4nderlust Awaiting requested review from w4nderlust w4nderlust is a code owner

@tgaddair tgaddair Awaiting requested review from tgaddair tgaddair is a code owner

@geoffreyangus geoffreyangus Awaiting requested review from geoffreyangus geoffreyangus is a code owner

@jeffkinnison jeffkinnison Awaiting requested review from jeffkinnison jeffkinnison is a code owner

@Infernaught Infernaught Awaiting requested review from Infernaught Infernaught is a code owner

@alexsherstinsky alexsherstinsky Awaiting requested review from alexsherstinsky alexsherstinsky is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vijayi1 @justinxzhao @arnavgarg1
0