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

install: Use amd maintained base for mpi/ucx/... #2098

Closed
wants to merge 6 commits into from

Conversation

mloubout
Copy link
Contributor

To be tested but that should work (it build locally at least)

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #2098 (62548d2) into master (d0e705d) will decrease coverage by 24.79%.
The diff coverage is n/a.

❗ Current head 62548d2 differs from pull request most recent head cebcaa1. Consider uploading reports for the commit cebcaa1 to get more accurate results

@@             Coverage Diff             @@
##           master    #2098       +/-   ##
===========================================
- Coverage   87.80%   63.02%   -24.79%     
===========================================
  Files         221      142       -79     
  Lines       38665    21695    -16970     
  Branches     5000     3924     -1076     
===========================================
- Hits        33951    13673    -20278     
- Misses       4164     7314     +3150     
- Partials      550      708      +158     

see 188 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@FabioLuporini FabioLuporini left a comment

Choose a reason for hiding this comment

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

I love this. Let me test it

@mloubout
Copy link
Contributor Author

mloubout commented Apr 1, 2023

It gives me some weird permission error locally not sure why but yeah seem like the "nicer" way to do it without caring about maintaining a base

@mloubout mloubout force-pushed the amd-docker-base branch 3 times, most recently from 424e9d3 to 37777c0 Compare April 3, 2023 17:29
@mloubout mloubout force-pushed the amd-docker-base branch 2 times, most recently from 68e540b to 4b2d1f6 Compare April 3, 2023 18:18
@mloubout mloubout force-pushed the amd-docker-base branch 2 times, most recently from 9b04065 to 4e17a4b Compare April 4, 2023 02:20
@FabioLuporini FabioLuporini changed the title docker: Use amd maintained base for mpi/ucx/... install: Use amd maintained base for mpi/ucx/... Apr 13, 2023
'-Xopenmp-target=amdgcn-amd-amdhsa']
self.ldflags += ['-march=%s' % platform.march]
self.ldflags += ['-fopenmp']
self.ldflags += ['--offload-arch=%s' % platform.march]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -701,7 +701,7 @@ def march(cls):
# mygpu will only print values accepted by cuda clang in
# the clang argument --cuda-gpu-arch.
try:
p1 = Popen(['mygpu', '-d', 'gfx900'], stdout=PIPE, stderr=PIPE)
p1 = Popen(['/opt/rocm-5.4.2/llvm/bin/offload-arch'], stdout=PIPE, stderr=PIPE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mloubout
Copy link
Contributor Author

Superseeded by #2104, closing

@mloubout mloubout closed this Apr 17, 2023
@mloubout mloubout deleted the amd-docker-base branch May 4, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants