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

Unit tests are failing #18

Closed
valosekj opened this issue Dec 6, 2024 · 8 comments
Closed

Unit tests are failing #18

valosekj opened this issue Dec 6, 2024 · 8 comments

Comments

@valosekj
Copy link
Member

valosekj commented Dec 6, 2024

After proposing some new changes in #17, I noticed that unit tests are failing.

The reason is these two commits 54ead73 and 2bad1f2 from @naga-karthik pushed directly to main. @naga-karthik, it seems these two commits are specific to your MS lesion project. Maybe you can create a new branch with these two commits, and I will revert them from main to unit tests working.

@naga-karthik
Copy link
Member

ah man, thanks for catching this! yeah I remember pushing directly to main, I am going to change it

@naga-karthik
Copy link
Member

hey Jan, I looked at these commits. I believe commit 54ead73 was a hasty fix to test something quick and I should have reverted it back. This commit skips the cases where EmptyRef and EmptyPred are true (i.e. the model has learned correctly and the Dice=1). I wanted to compare ANIMA and MetricsReloaded to ensure that the metrics are computed correctly (i.e. the results match in both ways of computing metrics).

Now, this commit can be reverted without having it another branch because this is precisely what we use MetricsReloaded for -- to not skip cases where GT is empty.

@naga-karthik
Copy link
Member

As for commit 2bad1f2, I see that the condition is commented out. I believe that was also to test something quick and should have been reverted back. I confirm the correct version is indeed seen in my branch (i.e. where it is not commented).

@naga-karthik
Copy link
Member

I did git reset --hard to commit 76dbb55, please let me know if: (1) this is right thing to do? (2) should add revert commits instead?

@valosekj
Copy link
Member Author

valosekj commented Dec 9, 2024

Thanks, Naga!

Yes, I believe you can run the following two commands on your local main:

git reset --hard 76dbb55
git push --force

@naga-karthik
Copy link
Member

thanks for the git push --force, Jan! I was missing this part! I am getting this error

remote: - Cannot force-push to this branch

This says that we need to allow force pushes from the Settings on Github. Do you have access by any chance?

@valosekj
Copy link
Member Author

valosekj commented Dec 9, 2024

Yes, I have access to the Settings. I temporarily disabled allow force pushes and ran:

git reset --hard 76dbb55
git push --force

Then I turned on the allow force pushes option again.

Now, the main branch is back to 76dbb55:

image

@naga-karthik
Copy link
Member

naga-karthik commented Dec 9, 2024

Merci, Jan! ❤️

Sorry for the mess, I will be careful with the main branch in the future. I recall the changes were quite "urgent".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants