8000 Added OpenCL compiler flags for Apple. by PStegmann · Pull Request #279 · adda-team/adda · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Added OpenCL compiler flags for Apple. #279

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

Merged
merged 2 commits into from
Nov 11, 2020

Conversation

PStegmann
Copy link

Description

Two minor changes have been implemented with this PR:

  • OpenCL compiler flags for Apple macOS Catalina
  • Adding .DS_Store to .gitignore to remove clutter for Apple developers.

Testing the executable provides the following result:

$ time ./adda_ocl -gpu 1
all data is saved in 'run001_sphere_g16_m1.5'
box dimensions: 16x16x16
lambda: 6.283185307   Dipoles/lambda: 15
Required relative residual norm: 1e-05
Total number of occupied dipoles: 2176
Searching for OpenCL devices
Using OpenCL device, based on Apple.
Device memory: total - , maximum object - 
Calculating Green's function (Dmatrix)
Fourier transform of Dmatrix......
Initializing clFFT
Total memory usage: 1.1 MB
OpenCL memory usage: peak total - 4.4 MB, maximum object - 1.5 MB

here we go, calc Y

CoupleConstant: 0.005259037197+1.843854148e-05i
x_0 = 0
RE_000 = 1.0000000000E+00
RE_001 = 8.4752662637E-01  + 
RE_002 = 8.2113292044E-01  + 
RE_003 = 4.2639057157E-01  + 
RE_004 = 3.0639031825E-01  + 
RE_005 = 2.2448283848E-01  + 
RE_006 = 2.2740255145E-01  - 
RE_007 = 1.5952149116E-01  + 
RE_008 = 6.1602401377E-02  + 
RE_009 = 3.5443250605E-02  + 
RE_010 = 2.9898244022E-02  + 
RE_011 = 2.4111231314E-02  + 
RE_012 = 3.8307716444E-03  + 
RE_013 = 3.0434605427E-03  + 
RE_014 = 1.3101400515E-03  + 
RE_015 = 8.2588869878E-04  + 
RE_016 = 5.0452779402E-04  + 
RE_017 = 1.1339754530E-04  + 
RE_018 = 9.3640493818E-05  + 
RE_019 = 6.8748674555E-05  + 
RE_020 = 2.2386140050E-05  + 
RE_021 = 1.5169630211E-05  + 
RE_022 = 3.1675098706E-06  + 
Cext	= 135.0449045
Qext	= 3.791149606
Cabs	= -2.047466079e-16
Qabs	= -5.747903078e-18

real	0m0.787s
user	0m0.083s
sys	0m0.036s

Related issues

No issues were created prior to this PR. Changes were discussed via email.

Types of changes

What types of changes does your code introduce to ADDA? Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. This is simply a reminder of what we are going to look for before merging your code.

  • I have read the contributing guidelines
  • Code compiles correctly
  • Tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works. And these tests pass
  • I have added/extended necessary documentation (if appropriate)

Further comments

This is a very small change.
On macOS make also cannot link against libgfortran.a out of the box, but I don't know how to fix that without hard-coding the gfortran directory in the makefile with -L/<location 8000 of libgfortran.a> and thus didn't include it in the PR.

@myurkin myurkin added this to the 1.4 milestone Nov 10, 2020
@myurkin myurkin self-assigned this Nov 10, 2020
@myurkin myurkin added comp-Scripts Related to Makefiles, developer and testing scripts maintainability Simplifies further code development (standardization, robustness) OpenCL Running on GPUs and similar devices OS-OSX Related to MacOS (Apple) pri-High Of higher priority, but no guarantees labels Nov 10, 2020
@myurkin
Copy link
Member
myurkin commented Nov 10, 2020

First, a brief note for reference. -framework is an option to Darwin (iOS system) linker, which is only described at its man page, e.g. this - but that probably implies that it will work with any compiler. GCC itself lists only -iframework, which is different.

Second, I have a question about libgfortran - can it be achieved with packages? I guess that is the default way and it works for clFFT and clBLAS, isn't it? @PStegmann

src/ocl/Makefile Outdated
@@ -80,8 +80,14 @@ endif
# !!! End of control section. Everything below is not designed to be modified by user
#=======================================================================================================================

UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Darwin)
LDLIBS += -framework OpenCL
Copy link
Member

Choose a reason for hiding this comment

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

Please make two-spaces indent. The same two lines below.

The rest is fine - ready for merge.

Copy link
Author

Choose a reason for hiding this comment

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

Done. I didn't pay attention to the auto-indent.

@myurkin
Copy link
Member
myurkin commented Nov 10, 2020

@PStegmann , while we are at it - have you added any library-link flags (-l..., like -lquadmath) to the Makefile for Fortran compilation? And which gcc version are you using?

I am asking because I have to add it on Windows combined with recent gcc, but not on Linux with gcc 4.8.5. Thus, want to devise a general solution.

8000
@PStegmann
Copy link
Author

which gcc version are you using?

I was using Homebrew gcc 10.2.0 and OpenCL 1.2 for the compilation and the test.

have you added any library-link flags (-l..., like -lquadmath) to the Makefile for Fortran compilation?

Aside from the flag for libgfortran.a I didn't have to change anything for macOS Catalina.

Second, I have a question about libgfortran - can it be achieved with packages?

I have to admit that I haven't worked with packages before. Are you planning to include the library in adda?

@myurkin
Copy link
Member
myurkin commented Nov 11, 2020

I have to admit that I haven't worked with packages before. Are you planning to include the library in adda?

I meant that Homebrew package (or module, not sure how it's called) for gcc or gfortran (if a separate one) should probably add some environment variables to make -L<path to libgfortran.a> unnecessary (similar to clFFT package and others). However, it may be the case that this works out of the box only if gfortran is used for linking (not only for compilation). And this didn't help us since we are linking C and Fortran sources together. So it can be that we are here in some gray zone (not clear who is responsible for this flag). Still, if there are some package like Fortran-dev, you could try it.

Concerning ADDA - not sure that I properly understood your question. We do link with Fortran libraries (since there is some Fortran sources), but do not plan to make ADDA operate as a library in the near future (#200).

@myurkin myurkin merged commit 22cd414 into adda-team:master Nov 11, 2020
@PStegmann
Copy link
Author

Ah, yes I am using the homebrew package for this. I suspect the cause of the problem might be that the C source is compiled with the native clang compiler and the Fortran source uses the homebrew gfortran.

Concerning ADDA - not sure that I properly understood your question. We do link with Fortran libraries (since there is some Fortran sources), but do not plan to make ADDA operate as a library in the near future (#200).

I just wasn't fully clear about what you meant by packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp-Scripts Related to Makefiles, developer and testing scripts maintainability Simplifies further code development (standardization, robustness) OpenCL Running on GPUs and similar devices OS-OSX Related to MacOS (Apple) pri-High Of higher priority, but no guarantees
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0