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

docker: add some tweaks to nvidia docker #2171

Merged
merged 3 commits into from
Aug 1, 2023
Merged

Conversation

mloubout
Copy link
Contributor

@mloubout mloubout commented Jul 25, 2023

From @kenhester suggestions

Should fix issues with #2023 as cupy-cuda12x seems to install and work fine inside the image

To be discussed, do we keep nvidia-clang in CI

@mloubout mloubout added CI continuous integration installation labels Jul 25, 2023
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #2171 (091dbb8) into master (44ee680) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2171      +/-   ##
==========================================
- Coverage   87.03%   86.92%   -0.12%     
==========================================
  Files         223      223              
  Lines       39969    39969              
  Branches     7302     7302              
==========================================
- Hits        34787    34742      -45     
- Misses       4600     4650      +50     
+ Partials      582      577       -5     
Files Changed Coverage Δ
devito/finite_differences/derivative.py 96.51% <100.00%> (ø)
devito/ir/iet/nodes.py 92.05% <100.00%> (ø)
devito/ir/support/space.py 90.55% <100.00%> (ø)
devito/passes/clusters/aliases.py 97.62% <100.00%> (ø)
devito/symbolics/inspection.py 91.48% <100.00%> (ø)
devito/types/dimension.py 94.32% <100.00%> (ø)
tests/test_builtins.py 100.00% <100.00%> (ø)
tests/test_derivatives.py 100.00% <100.00%> (ø)
tests/test_dimension.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@FabioLuporini
Copy link
Contributor

To be discussed, do we keep nvidia-clang in CI

Probably we can drop it

NVC for openmp offloading on NVidia cards
Amdclang for openmp offloading on AMD cards
icx for openmp offloading on Intel cards

That's completely fine by me

IIRC, we're still stuck with our own fork of clang, so one more reason to drop it.

Can we try again NVC + openmp offloading, now that we finally have NVC 23.5 ?

Copy link
Contributor

@georgebisbas georgebisbas left a comment

Choose a reason for hiding this comment

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

I would vote for dropping as well...not popular, not faster, cumbersome for non-docker....
But will we also drop related infrastructure?

@mloubout
Copy link
Contributor Author

Can we try again NVC + openmp offloading

Yes

Ok removing clang build and replacing by nvc

@mloubout mloubout force-pushed the nvidia-docker-misc branch 5 times, most recently from 645309c to 34cfdf4 Compare July 27, 2023 14:54
@mloubout mloubout force-pushed the nvidia-docker-misc branch 2 times, most recently from 8a28529 to 36c6715 Compare July 28, 2023 13:58
@mloubout mloubout force-pushed the nvidia-docker-misc branch 4 times, most recently from c884a9d to ec38066 Compare July 31, 2023 16:32
flag: '--gpus all'
- base: 'bases:nvidia-nvc'
tag: 'nvidia-nvc-omp'
flag: '--gpus all --env DEVITO_LABGUAGE=openmp'
Copy link
Contributor

Choose a reason for hiding this comment

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

LAN*

@mloubout mloubout force-pushed the nvidia-docker-misc branch 8 times, most recently from f8e0721 to 5ac55c9 Compare August 1, 2023 14:52
@mloubout mloubout merged commit 6f75a9e into master Aug 1, 2023
35 checks passed
@mloubout mloubout deleted the nvidia-docker-misc branch August 1, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI continuous integration installation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants