Goddard-Fortran-Ecosystem / pFUnit

Parallel Fortran Unit Testing Framework
Other
169 stars 45 forks source link

Port to Intel Visual fortran on Windows #286

Open mbraakhekke opened 3 years ago

mbraakhekke commented 3 years ago

I have managed to build pFUnit on Windows with Intel Visual Fortran. It required a few tweaks to pFUnit and the external dependencies, fArgParse, gFTL-shared, and gFTL. The main changes involve changing the compiler flags, which are different under Windows. Would you be willing to integrate these changes? If so I can submit PRs.

tclune commented 3 years ago

I will definitely accept such changes. Note though though that I have no means of testing under Windows, so it will be incumbent on users such as yourself to let me know when other changes unintentionally break under Windows.

Cheers.

mbraakhekke commented 3 years ago

OK, great! And I'd be happy to test new versions on Windows, for as long as I have access to Intel Visual Fortran--which will hopefully be the case in the foreseeable future.

mbraakhekke commented 3 years ago

I've been working on the porting off and on during the last weeks. I thought I'd give an update. It turned out to be a lot more complicated than I thought. Below an overview of 4 issues that I('m tying to) tackle(d). Could take a look and if possible comment? Points 1 and 2 are truly necessary for porting to Intel Visual Fortran on Windows. Points 3 and 4, however are not so I propose to submit these changes in separate PRs.

1. Compiler options

This was imply a matter of changing the Intel specific compiler flags in the relevant CMake files.

2. Generated sources and build dependencies

Intel Visual Fortran has its own dependency-manager that scans the files just before the build. Several of the targets of pFUnit (fhamcrest, assert) have generated sources that import modules defined in other files. Since the generation and the building happens in the same target, the generated files don't exist at the start of the build. So the dependency manager can't scan these, which results in incorrect compile order and missing module errors (see also my post on the CMake forum).

I see several possible solutions:

  1. in the depender files include the files that the define the imported (dependee) modules, either as a Fortran include or preprocessor #include. See my post on the Intel forum. Since the generated and non-generated files are in different directories, the directory containing the non-generated sources would need to be added to the include directories of the target. I haven't figured out how to do that with CMake yet. But when I do it manually, it works. The only thing is that some dependee modules are imported by multiple depender modules and I need to add the include line in each depender file. This results in linker warnings about multiple included files. I've tried solve this with include guards but haven't managed so far.

  2. Create a separate target for generating the files, which would be run first. The actual building would happen after that in a separate target ("project" in VS). The generated files then exist so the dependencies can be resolved.

  3. Generate the files during the configuration by CMake.

Do you have a preference regarding any of these solutions? I think option 2 would probably be easiest to implement. And should I implement these changes for all systems or would you want me to limit them to Windows/VS?

3. M4 tool

The gFTL build uses M4, which is not standard available on Windows. There's a port from the GnuWin32 project. Since downloading it and setting the required variable is a hassle, I've changed the CMakeLists to do that for the user. The only thing is that the downloading only works when running CMake from the command line (see this issue); not when using CMake-gui. I plan to get back to that later.

4. Compilation of trial sources

Both pFUnit and gFTL-shared use trial sources during the CMake configuration. On Windows/VS this is extremely slow. As a workaround I modified the relevant CMake files to do the compiling and running mannualy using execute_process(). This speeds things up a little. I would also propose to put the relevant variables obtained by running the trial sources in the CMake cache so it doesn't redone every time the CMake configuration is rerun.

tclune commented 3 years ago

Regarding the generated sources, I have to admit to being relatively surprised that CMake + VisualStudio has an issue on this front. Lots of CMake packages use generated files.

I'm not sure I understand howe your option #2 (separate target) would help. Presumably even on windows the cmake phase is separate from the actual build phase. How would you build the first target without completing CMake's own dependency analysis? Or does CMake just skip the dependency analysis when using VisualStudio? (But even that would seem to just create a different issue.)

Option #3 seems like it would be relatively straightforward but I can imagine that there are wrinkles that I don't immediately see.

Regarding M4 - just so long as the logic is relatively clear and does not change the behavior under unix.

Regarding compilation of trial sources: for most user this is a do-it-once-and-forget step. Is the performance really so slow that we need to risk adding complexity? But I definitely agree that the result variables should be put in cache to avoid repeated execution.

Please consider handling these with separate PRs where possible. Esp. the variable caching.

mbraakhekke commented 3 years ago

I'm not sure I understand howe your option #2 (separate target) would help. Presumably even on windows the cmake phase is separate from the actual build phase. How would you build the first target without completing CMake's own dependency analysis? Or does CMake just skip the dependency analysis when using VisualStudio? (But even that would seem to just create a different issue.)

Yes, with Windows/VS, the CMake phase is separate from building. Each CMake target becomes a "VS project" that is built separately. Dependencies between projects are explicitly specified (CMake does this based on the specified target_link_libraries). But dependencies within a project (i.e. between source files) are handled by Intel's own dependency analysis. (NB: this is based on my own deductions, so I might be wrong). So by creating a separate target/project for the generation of the sources and making this a dependency of the the second target in which the sources are built the problem would be solved, I think. Because when building of the second target/project starts, the generated files exist and can be scanned for dependencies.

How about I implement and see if it works? Should not be too hard, I think.

Regarding M4 - just so long as the logic is relatively clear and does not change the behavior under unix.

Yeah, its a relatively minor change that would be enclosed in a if (WIN32) clause.

Regarding compilation of trial sources: for most user this is a do-it-once-and-forget step. Is the performance really so slow that we need to risk adding complexity? But I definitely agree that the result variables should be put in cache to avoid repeated execution.

I haven't timed it but I think building everything ended up taking more than 10 min. But the speed gains by doing it manually are not exactly shocking. Part of the problem might also be that we use a floating license for our compiler so with every call to ifort involves communicating with the license server. So I agree it might not be worth it. I'll make the changes to store the result variables in the CMake cache.

tclune commented 3 years ago

OK - I think I understand now regarding the handling of targets under windows.

mbraakhekke commented 3 years ago

I'm read to submit the PRs but with respect to gFTL I'm a bit unsure how to proceed. I'm currently mainly interested in pFUnit, which indirectly links to an older commit of gFTL: Goddard-Fortran-Ecosystem/gFTL@34bbeddd9192332258f820ea2ae06772552d54e6. So if the PR gets merged in to the head, it won't affect pFUnit. Is it possible to incorporate the PR retroactively into 34bbedd? Or are you planning to update the gFTL submodule of gFTL-shared to the head any time soon? (If so, you may run into problems because but pFUnit and gFTL have a target named tests--I tried)

tclune commented 3 years ago

@mbraakheke Before I can accept any PRs I'll need a signed CLA for each of the respective repositories. (Apologies - this is a relatively new process that the NASA lawyer have established.)

The basic process then is to do a PR onto the develop branch of gFTL. I'll then roll that into main and do a new release there. I can then go in and update gFTL-shared and fArgParse to point at the latest gFTL. I can then update pFUnit (develop) to use the latest fArgparse, and then accept your change on top of that. A bit tedious, but the nesting used to be even worse.

mbraakhekke commented 3 years ago

@mbraakheke Before I can accept any PRs I'll need a signed CLA for each of the respective repositories. (Apologies - this is a relatively new process that the NASA lawyer have established.)

No problem.

The basic process then is to do a PR onto the develop branch of gFTL. I'll then roll that into main and do a new release there. I can then go in and update gFTL-shared and fArgParse to point at the latest gFTL. I can then update pFUnit (develop) to use the latest fArgparse, and then accept your change on top of that. A bit tedious, but the nesting used to be even worse.

That's clear but I still need to know if the changes in the PR for gFTL should be relative to the HEAD or the commit linked to by gFTL-shared.

tclune commented 3 years ago

Sorry for the misunderstanding. My preference would be for the changes to gFTL to be relative to its develop branch. The only development in the last several months has been for the v2 interfaces, so I doubt there will be any conflicts when you rebase from wherever you started.

mbraakhekke commented 3 years ago

I've managed to use the built-in "test explorer" of Visual Studio with the tests created with pFUnit--see pic.. It requires a plugin (CTestAdapter) and it's not exactly perfect, but still pretty cool. I hope to expand the functionality.

pFUnit_VS_example

mbraakhekke commented 2 years ago

For various reasons this work was interrupted for a few months. I've picked it up again and will submit the PRs.