fortran-lang / fpm

Fortran Package Manager (fpm)
https://fpm.fortran-lang.org
MIT License
884 stars 99 forks source link

ifort is generating threaded coarray code by default #283

Closed LKedward closed 3 years ago

LKedward commented 3 years ago

I think the ifort coarray flag should be -coarray=single to match the default behaviour we have for gfortran.

Currently we have: https://github.com/fortran-lang/fpm/blob/6e46fba605e43473e7d7f680f13ca8ff02ec436e/fpm/src/fpm_compiler.f90#L139-L144

which I believe will default to generating shared-memory coarray code.

Like OpenMP, coarrays should be enabled explicitly by a build option (https://github.com/fortran-lang/fpm/issues/112#issuecomment-723652236), except in the case of the caf compiler wrapper.

ivan-pi commented 3 years ago

This would explain the results seen here: https://fortran-lang.discourse.group/t/the-counter-intuitive-rise-of-python-in-scientific-computing/469/7?u=ivanpribec

urbanjost commented 3 years ago

Well, that explains a lot. Certainly should be changed, especially since ifort has more back doors for specifying compiler switches than any other compiler I know of. Unlike most of the others using fpm an ifort compiler user can set additional compiler flags via config files (see $IFORTCFG) relatively easily. I tested with > 20 packages and did not catch that. I think I will add a mini-version of his code into my QA list of fpm packages; which raises the thought of whether we should have a shared set of fpm repositories and a quick basic test of them (or do we, and I do not know about it?). And I have ifort now; just in time for the new Intel suite to be available free. I think just running the test with gfortran is sufficient for now. With the alpha out in the wild I'd like this picked off quickly. I cannot make a PR right now; but will in about six hours if no one else has.

everythingfunctional commented 3 years ago

I'm trying out ifort since it was just released for free, but nowhere in their help is the option -coarray mentioned, and I get the message ifort: command line warning #10006: ignoring unknown option '-coarray=single'. However, if I add the flag -warn all before it (but not after) I don't get that message. Anybody else have ideas for this?

everythingfunctional commented 3 years ago

I think I figured it out. I'm on MacOS, where apparently coarrays aren't supported.

error #8347: Coarray is not supported on this platform.   [SUITE_FAILED]
jvdp1 commented 3 years ago

I am not aware that iort support something like -coarray=single. AFAIK -coarray only accept shared (i.e. -coarray=shared) or distributed (-coarray=distributed) However, the following could maybe do the trick:

ifort -coarray=shared -coarray-num-images=1

urbanjost commented 3 years ago

At least in newer versions it should take shared|distributed|coprocessor|single on the "Classic" ifort. Not sure out the ifx one. If one of them is not present it will reject coarray syntax. =single is supposed to be for debugging where it does not actually spawn multiple images but allows the syntax, which does not quite jive with what everyone is seeing. Are we talking the classic or new Intel compiler? The new distribution comes with both.

urbanjost commented 3 years ago

If it literally is causing failures on any platform we need to take it out and for now Intel users can use a config file to enable it and hopefully soon additional flags will be supported in the TOML file and perhaps with --fflags|--flags on the fpm command line; but having coarray syntax be illegal by default is not very appealing so I am hoping -coarray=single is good everywhere and from the documentation I thought it would be; but that does not seem to be the case?

everythingfunctional commented 3 years ago

On MacOS it just issues a warning (sometimes) and ignores any -coarray flag. However, any code with coarray features cannot be compiled with ifort on MacOS.

LKedward commented 3 years ago

which raises the thought of whether we should have a shared set of fpm repositories and a quick basic test of them

This is a good point and could be a useful to add to our automated CI checks.

LKedward commented 3 years ago

Fixed by #300