Reintroduce Fortran compiler options in setup script

[chjacob-tubs]

The fortran compiler is used by default, so this option is needed to compile sucessfully if you dont use the default compiler.

[codecov-io]

Codecov Report

Merging #62 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #62   +/-   ##
=======================================
  Coverage   42.29%   42.29%           
=======================================
  Files          78       78           
  Lines        1785     1785           
=======================================
  Hits          755      755           
  Misses       1030     1030

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7e3fe55...ec0cd84. Read the comment docs.

[bast]

Should the Fortran part not be optional? It was and possibly it now became default.

Anyway, this change is in principle good but we should still modify it. Setup script is now autogenerated. So this risks being wiped in future. Also it is missing some CMake code to go with it. It is better to modify https://github.com/dftlibs/xcfun/blob/master/cmake/autocmake.yml#L22, add fc in there and regenerate setup and CMakeLists.txt by running python update.py ... You will need the pyyaml package in your virtual environment or similar. More context: http://autocmake.readthedocs.io/en/latest/developers/faq.html#which-files-do-i-need-to-edit

[robertodr]

I second what Radovan said. The reason why I did not put the autogeneration of that command in is that the Fortran compiler is only needed to compile the test case. The Fortran bindings in the xcfun.f90 file are not compiled anymore into the library as that would require installing a machine- and compiler-dependent .mod file. It is now up to the end-user to grab that file and compile with his/her sources before linking to XCFun.

Also, the place where Fortran things now live is very localized and it made it a bit harder to get it parsed through Autocmake.

[bast]

If possible I would make Fortran support optional (it possibly is the situation now, not sure). This is because there are codes that don't need it and systems that don't have Fortran and I would not like to force codes to require Fortran in case they don't use it.

[robertodr]

It is fully optional as there are no Fortran components in the library itself, only a test case which is compiled independently and on-demand. xcfun.f90 will always be installed, but that's just copying a file around.

[chjacob-tubs]

I understand the auto-generated part can can try to change that.

About the need for this change: Yes, the F90 interface is optional now / only compiled for the tests, I can see that. However, it is activated by default. And if it is active, I need a way to specify the Fortran compiler. Currently, I cannot do this, because the corresponding option of setup is gone.

This is all that I want: An option to specify the Fortran compiler in case a Fortran compiler is needed of building xcfun

[robertodr]

Ooops, sorry. That might have slipped through the cracks when I was testing. Should it then be that TEST_Fortran_API is OFF by default? Maybe only turned ON when a Fortran compiler is explicitly specified?

[chjacob-tubs]

I think one of the cmake experts has to take over here ...

To me, any solution that works is fine.

[bast]

We could use this one: https://github.com/coderefinery/autocmake/blob/master/modules/fc_optional.cmake

[bast]

Present situation:

• TEST_Fortran_BINDINGS is ON by default
• this enables Fortran support
• currently there is no obvious way to specify the Fortran compiler then

Suggested changes:

• I think we should make Fortran optional
• we can use https://github.com/coderefinery/autocmake/blob/master/modules/fc_optional.cmake
• testing the bindings can be another toggle but I would set it ON if Fortran is enabled
• in any outcome we need to make it possible to change the Fortran compiler via the setup script
• EDIT Fortran support is OFF by default

[bast]

I can take care of the changes but what is your view Roberto?

[robertodr]

I agree with your assessment:

• the Fortran compiler has to be optional
• TEST_Fortran_BINDINGS is OFF unless a Fortran compiler is passed, thus enabling the language. Tethering the two together means that TEST_Fortran_BINDINGS can actually be removed as an option.

[bast]

I am working on this ...

[bast]

My take on this is presented in #65.

[bast]

This has been solved in #65. Closing this PR. But thanks for bringing it up.