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

rename IoU -> Jaccard Index #662

Merged
merged 11 commits into from
Dec 6, 2021
Merged

rename IoU -> Jaccard Index #662

merged 11 commits into from
Dec 6, 2021

Conversation

Borda
Copy link
Member

@Borda Borda commented Dec 6, 2021

What does this PR do?

Preventing future collision with adding detection IoU metrics
and in docs we say that our IoU is known as jaccard index anyway

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?

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 🙃

@Borda Borda added the refactoring refactoring and code health label Dec 6, 2021
@Borda Borda requested a review from SkafteNicki December 6, 2021 11:17
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #662 (ed9b335) into master (4e26593) will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #662   +/-   ##
=====================================
  Coverage      95%    95%           
=====================================
  Files         162    164    +2     
  Lines        5984   6002   +18     
=====================================
+ Hits         5696   5714   +18     
  Misses        288    288           

@Borda Borda marked this pull request as ready for review December 6, 2021 11:47
@Borda Borda added this to the v0.7 milestone Dec 6, 2021
@Borda Borda enabled auto-merge (squash) December 6, 2021 13:48
@mergify mergify bot removed the has conflicts label Dec 6, 2021
@Borda Borda merged commit 965a6cf into master Dec 6, 2021
@Borda Borda deleted the refactor/iou branch December 6, 2021 18:22
weiji14 added a commit to weiji14/ctcorenet that referenced this pull request Nov 1, 2024
Fix the following errors

- AttributeError: type object 'Trainer' has no attribute 'add_argparse_args' (need to change from Pytorch Lightning 1.0 to Lightning 2.0 style, see Lightning-AI/pytorch-lightning#19905)
- ValueError: Cannot find 8ef2e2d423b67b53ec8113fc71a9b968bb0f66e7 in https://github.com/mateuszbuda/brain-segmentation-pytorch (change to use v1.0 tag)
- AttributeError: module 'torchmetrics' has no attribute 'IoU' (renamed to JaccardIndex, see Lightning-AI/torchmetrics#662)

Also gitignoring __pycache__/ folder.
weiji14 added a commit to weiji14/ctcorenet that referenced this pull request Nov 3, 2024
* 📝 Add installation instructions and update Binder link

Some more getting started instructions to install the package, and updated the quickstart button from Pangeo Binder to regular Binder.

* ⬆️ Bump pytorch and other dependencies

Upgrading pytorch, torchvision and other dependencies to newer versions in 2024. Simplified the conda environment.yml file to use conda-forge channel only and remove the md5 hash (put in the lock file instead). Not installing black and codecarbon as required dependencies anymore. Installing ONNX so model export will work.

* 👽 Update CTCoreUnet to handle newer pytorch* library versions

Fix the following errors

- AttributeError: type object 'Trainer' has no attribute 'add_argparse_args' (need to change from Pytorch Lightning 1.0 to Lightning 2.0 style, see Lightning-AI/pytorch-lightning#19905)
- ValueError: Cannot find 8ef2e2d423b67b53ec8113fc71a9b968bb0f66e7 in https://github.com/mateuszbuda/brain-segmentation-pytorch (change to use v1.0 tag)
- AttributeError: module 'torchmetrics' has no attribute 'IoU' (renamed to JaccardIndex, see Lightning-AI/torchmetrics#662)

Also gitignoring __pycache__/ folder.

* ⬆️ Bump setup-miniconda from 2.1.1 to 3.0.4 and other Actions

Bumps [conda-incubator/setup-miniconda](https://github.com/conda-incubator/setup-miniconda) from 2.2.0 to 3.0.4.
- [Release notes](https://github.com/conda-incubator/setup-miniconda/releases)
- [Changelog](https://github.com/conda-incubator/setup-miniconda/blob/main/CHANGELOG.md)
- [Commits](conda-incubator/setup-miniconda@v2.2.0...v3.0.4)

Also bumped actions/checkout from v2.3.4 to v4.2.2, and ubuntu from 20.04 to 24.04. Commented out `black` code quality check for now.

* 🛂 Remove sha256 hashsum in explicit lockfile

Getting this error on CI: `ParseError: Could not parse explicit URL: https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2#fe51de6107f9edc7aa4f786a70f4a883943bc9d39b3bb7307c04c41410990726`. Try without the sha256 hashsum suffix to see if setup-miniconda works.

* 🛂 Trust downloading UNet model from torch.hub.load

Assuming that mateuszbuda/brain-segmentation-pytorch is a trusted source to download the UNet model, so that Continuous Integration can work without a Y/N prompt.

* 🔇 Only log to tensorboard if activated

Fix `AttributeError: 'ExperimentWriter' object has no attribute 'add_scalar'` by gating the `self.logger.experiment.add_scalar` call behind an if-condition. Logging can be activated if tensorboard is installed, and the `--logger=True` flag is passed via the command-line.

* 🔍 Add teaser img of sediment core with IRD clasts to main README.md

Include a teaser image of a sediment core with Ice-Rafted Debris (IRD) clasts highlighted in red to the main README.md file. Also fixed a small typo.

* 📌 Use CUDA build of pytorch instead of CPU build

Re-created conda environment by running `CONDA_OVERRIDE_CUDA=12.6 conda env create --file=environment.yml --solver=libmamba` and re-locking with `conda list --explicit > environment-linux-64.lock`. Unsure why it's giving us CUDA 11.8 instead of CUDA 12, but oh well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready refactoring refactoring and code health
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants