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

ci: proper setting oldest/latest for integrations & PL>=1.9 #2129

Merged
merged 37 commits into from
Oct 17, 2023
Merged

Conversation

Borda
Copy link
Member

@Borda Borda commented Oct 2, 2023

What does this PR do?

Integrations on GPU were strangely set always oldest req.

Dropped compatibility with:

  • 1.6 - no special reason, but the next two versions wont work so skip also this old one
  • 1.7 - Pl was looking for removed protected function, ERROR integrations/test_lightning.py - ImportError: cannot import name '_compare_version' from 'torchmetrics.utilities.imports'
  • 1.8 - not aligned dependencies:
     pytorch-lightning 1.8.0 depends on lightning-utilities==0.3.*
     torchmetrics 1.3.0.dev0 depends on lightning-utilities<0.10.0 and >=0.8.0
    
Before submitting
  • Was this discussed/agreed 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 🙃


📚 Documentation preview 📚: https://torchmetrics--2129.org.readthedocs.build/en/2129/

@Borda Borda added the test / CI testing or CI label Oct 2, 2023
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #2129 (42e6fef) into master (09bd064) will increase coverage by 0%.
The diff coverage is 100%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #2129   +/-   ##
======================================
  Coverage      87%     87%           
======================================
  Files         293     293           
  Lines       16407   16415    +8     
======================================
+ Hits        14253   14261    +8     
  Misses       2154    2154           

@Borda
Copy link
Member Author

Borda commented Oct 2, 2023

@JustinGoheen @SkafteNicki seems the logging is different than expected for the latest PL (we have not tested it for long time 😿 ) mind have a look, pls: https://dev.azure.com/Lightning-AI/Metrics/_build/results?buildId=176883&view=logs&j=12a9e589-df1e-541c-f0d0-67a8d22922d8&t=b6423d8e-d265-5686-c4f1-df41b5bef40a&l=279

@Borda Borda changed the title ci: proper setting oldest for integrations ci: proper setting oldest/latest for integrations Oct 2, 2023
@Borda Borda added the Important milestonish label Oct 3, 2023
.azure/gpu-integrations.yml Outdated Show resolved Hide resolved
@SkafteNicki SkafteNicki added this to the v1.2.x milestone Oct 6, 2023
@Borda
Copy link
Member Author

Borda commented Oct 6, 2023

@awaelchli @justusschock, any ringing belts that could be the reason for the case failing...

@Borda Borda enabled auto-merge (squash) October 9, 2023 07:35
@Borda Borda mentioned this pull request Oct 9, 2023
4 tasks
@mergify mergify bot removed the has conflicts label Oct 11, 2023
@Borda Borda changed the title ci: proper setting oldest/latest for integrations [wip] ci: proper setting oldest/latest for integrations & PL>=1.9 Oct 11, 2023
@Borda Borda disabled auto-merge October 11, 2023 09:25
@mergify mergify bot removed the has conflicts label Oct 11, 2023
@Borda
Copy link
Member Author

Borda commented Oct 11, 2023

Setting the upper bound for pl<2.0 for GPU issues but need to be investigated and unfreeze

  • FAILED integrations/test_lightning.py::test_metric_lightning_log - AssertionError: assert -2.639833450317383 == tensor(-4.4507, device='cuda:0')
     >       assert epoch_0["metric_update_epoch"] == sum([model.sum[0], model.sum[1]])
     E       AssertionError: assert -2.639833450317383 == tensor(-4.4507, device='cuda:0')
     E        +  where tensor(-4.4507, device='cuda:0') = sum([tensor(3.3539, device='cuda:0'), tensor(-7.8046, device='cuda:0')])
    
  • FAILED integrations/test_lightning.py::test_metric_collection_lightning_log - assert False
     >       assert torch.allclose(tensor(logged["SumMetric_epoch"]), model.sum, atol=2e-4)
     E       assert False
     E        +  where False = <built-in method allclose of type object at 0x7fc6260c6a40>(tensor(-22.7884), tensor(-18.4362), atol=0.0002)
     E        +    where <built-in method allclose of type object at 0x7fc6260c6a40> = torch.allclose
     E        +    and   tensor(-22.7884) = tensor(tensor(-22.7884))
     E        +    and   tensor(-18.4362) = TestModel(\n  (layer): Linear(in_features=32, out_features=2, bias=True)\n  (metric): MetricCollection(\n    (SumMetric): SumMetric()\n    (DiffMetric): DiffMetric()\n  )\n).sum
    

@Borda Borda merged commit 045431a into master Oct 17, 2023
61 of 67 checks passed
@Borda Borda deleted the ci/oldest branch October 17, 2023 16:00
@mergify mergify bot added the ready label Oct 17, 2023
Borda added a commit that referenced this pull request Nov 30, 2023
Borda added a commit that referenced this pull request Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Important milestonish ready test / CI testing or CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants