Discussion about c-fortran interoperability

jhamman commented 9 years ago

@bartnijssen, @mabrunke, and @apcraig -

I have a first attempt at using the Standard Fortran and C Interoperability on the master branch of this repo. It is working on my local machine using the newest versions gcc and gfortran but is failing on the HPC machines that use the Intel or older versions of the GNU compilers. I'll explain why I think that is later and hopefully one of you will have an idea of why that is.

Code Design

The code I have essentially tracks the desired workflow for what we want to see in the VIC/CN coupling (see https://github.com/UW-Hydro/VIC/pull/174 for more information).

Memory is created and initialized in C using a C-structure: cn_data_struct.
A Fortran subroutine is developed that accepts the cn_data_struct as an argument
The Fortran subroutine is able to operate directly on the cn_data_struct without a copy.

Looking at the code in this example, here where each of those 3 steps are done:

The C function make_cn_struct creates and initializes an array of cn_data_structs.
The Fortran subroutine use_cn_data_struct defines a derived type vic_cn_data_type that matches the cn_data_struct. The variable is defined as intent(inout) so any changes are applied to the original memory location of cn_data_struct.
The Fortran subroutine use_cn_data_struct modifies the values of all members of the structure.

Outside of this example, it would probably make sense to put the definition of vic_cn_data_type in a MODULE so it would be available to other subroutines.

Now, the problem I mentioned earlier:

The cn is defined as an array of cn_data_structs, one for each elevation band. It seems like there is a problem passing arrays like this through the C-Fortran interoperability when you are using an assumed size or shape. This is the relevant line:

type(vic_cn_data_type), DIMENSION(:), intent(inout) :: cn_data

I'm sure there is a way to do this without assuming either the shape or the size but I wasn't able to get it to work. @mabrunke or @apcraig may have a better idea of how that could work. After all, I am passing all the relavent dimensions as arguments to the subroutine.

Useful links

mabrunke commented 9 years ago

Joe:

I was wondering if one could have an array of derived types in Fortran, and I didn't think it was going to work. I'm surprised that it did work in gnu. The CN code has none, and all of the arrays are elements of derived types. I would suggest changing the cn C structure so that the band dimension is attached to each element (i.e., adding a band dimension to each element). I suspect then that this wouldn't be a problem anymore.

--Michael

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

MICHAEL A. BRUNKE Research Specialist, Atmos. Sci. Phone: (520) 626-7349 Institute of Atmospheric Physics Fax: (520) 621-6833 The University of Arizona 1118 East Fourth Street brunke@atmo.arizona.edu P.O. Box 210081 http://www.u.arizona.edu/~brunke Tucson, AZ 85721-0081

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

On Mon, 19 Jan 2015, Joe Hamman wrote:

@bartnijssen, @mabrunke, and @tcraig -

I have a first attempt at using the Standard Fortran and C Interoperability on the master branch of this repo. It is working on my local machine using the newest versions gcc and gfortran but is failing on the HPC machines that use the Intel or older versions of the GNU compilers. I'll explain why I think that is later and hopefully one of you will have an idea of why that is.

Code Design

The code I have essentially tracks the desired workflow for what we want to see in the VIC/CN coupling (see https://github.com/UW-Hydro/VIC/pull/174 for more information).

Memory is created and initialized in C using a C-structure: cn_data_struct.

A Fortran subroutine is developed that accepts the cn_data_struct as an argument

The Fortran subroutine is able to operate directly on the cn_data_struct without a copy.

Looking at the code in this example, here where each of those 3 steps are done:

The C function make_cn_struct creates and initializes an array of cn_data_structs.

The Fortran subroutine use_cn_data_struct defines a derived type vic_cn_data_type that matches the cn_data_struct. The variable is defined as intent(inout) so any changes are applied to the original memory location of cn_data_struct.

The Fortran subroutine use_cn_data_struct modifies the values of all members of the structure.

Outside of this example, it would probably make sense to put the definition of vic_cn_data_type in a MODULE so it would be available to other subroutines.

Now, the problem I mentioned earlier:

The cn is defined as an array of cn_data_structs, one for each elevation band. It seems like there is a problem passing arrays like this through the C-Fortran interoperability when you are using an assumed size or shape. This is the relevant line:
type(vic_cn_data_type), DIMENSION(:), intent(inout) :: cn_data
I'm sure there is a way to do this without assuming either the shape or the size but I wasn't able to get it to work. @mabrunke or @tcraig may have a better idea of how that could work. After all, I am passing all the relavent dimensions as arguments to the subroutine.

Useful links

https://software.intel.com/en-us/node/510843

http://www.codeproject.com/Tips/456681/C-Fortran-Interoperability-With-External-Fortran-F

http://stackoverflow.com/questions/13739732/how-can-i-access-a-c-pointer-from-fortran

https://gcc.gnu.org/onlinedocs/gcc-4.8.4/gfortran/ISO_005fC_005fBINDING.html#ISO_005fC_005fBINDING

Reply to this email directly or view it on GitHub: https://github.com/jhamman/c-fortran-tests/issues/1

jhamman commented 9 years ago

The code on the master branch now works for both Intel and GNU compilers on Spirit.

Note, that if you looked at the code more than an hour ago, I've updated the way I pass the cn_data_struct. It is now a pointer that is converted to a proper array of derived types in Fortran.

@mabrunke, take another look. You should be able to run the code on any machine with GNU or Intel compilers. I don't have easy access to other compilers so I haven't tested beyond that. @bartnijssen, this addresses your concern as well.

mabrunke commented 9 years ago

Joe:

I think I was already looking at the version you posted 1/2 hour ago, so forget about what I commented earlier. If this works now, then it works.

--Michael

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

MICHAEL A. BRUNKE Research Specialist, Atmos. Sci. Phone: (520) 626-7349 Institute of Atmospheric Physics Fax: (520) 621-6833 The University of Arizona 1118 East Fourth Street brunke@atmo.arizona.edu P.O. Box 210081 http://www.u.arizona.edu/~brunke Tucson, AZ 85721-0081

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

On Tue, 20 Jan 2015, Joe Hamman wrote:

The code on the master branch now works for both Intel and GNU compilers on Spirit.

Note, that if you looked at the code more than an hour ago, I've updated the way I pass the cn_data_struct. It is now a pointer that is converted to a proper array of derived types in Fortran.

@mabrunke, take another look. You should be able to run the code on any machine with GNU or Intel compilers. I don't have easy access to other compilers so I haven't tested beyond that. @bartnijssen, this addresses your concern as well.

Reply to this email directly or view it on GitHub: https://github.com/jhamman/c-fortran-tests/issues/1#issuecomment-70721784

apcraig commented 9 years ago

Joe,

I have no experience with the c/fortran standard, but it looks great. Great work and if this works, it's very powerful. First, the issue of the allocation of cn_data on the fortran side. I think what you need is an allocation statement somewhere. Assumed size/shape is only relevant when an argument is passed to a subroutine. You have to explicitly declare the size somewhere. So, I think you might need to add

allocate(cn_data(Nbands))

before the call to c_f_pointer. That might fix your pointer problem. Let us know if that helps. It's not clear to me how to use the use_cn_data_struct subroutine. Could you point us to that code as well. Do you actually call into this routine, from fortran or c? do you call it just once to instantiate the pointers on the fortran side and then you use the datatype explicitly wherever you want on the fortran side? is the cn_data fortran type how you access the c side? so cn_data, in general, has to be public and accessible to other parts of the fortran code?

Separately, where does the "21" come from in the vic_cn_data_type declaration. Does that number match something in vic and it won't change. A few thoughts, is it possible to get that number from vic? Maybe we could allocate all those arrays on the fly. I would say that's a longer term implementation issue that should be deferred for now. Short term, how about if you implement a module parameter at the top and set it to 21 and use that instead of hardwiring 21 everywhere. That would look like

integer, parameter :: vsize = 21

and then substitute 21 for vsize. That will make it easier to change later if needed. I have no problem keeping the 21 as is if that's never expected to change. This is a minor detail in either case.

mabrunke commented 9 years ago

Tony:

21 = the total # of PFTs in CN. That will be mxpft + 1 (one more for bare soil, where mxpft is defined in clm_varpar.F90 in feature/CN of mabrunke github). I would suggest adding the definition of this derived type to those in clmtype.F90 and then the allocation as Tony suggests can happen when the other clmtype derived types are initialized in the call to clm_initialize2. I would also suggest that more variables may be needed to be added to the C cn structure so that only one structure is passed back and forth between VIC and CN. Right now, it just includes the data that CN needs included in the state files.

--Michael

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

MICHAEL A. BRUNKE Research Specialist, Atmos. Sci. Phone: (520) 626-7349 Institute of Atmospheric Physics Fax: (520) 621-6833 The University of Arizona 1118 East Fourth Street brunke@atmo.arizona.edu P.O. Box 210081 http://www.u.arizona.edu/~brunke Tucson, AZ 85721-0081

MMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMMM

On Tue, 20 Jan 2015, apcraig wrote:

Joe,

I have no experience with the c/fortran standard, but it looks great. Great work and if this works, it's very powerful. First, the issue of the allocation of cn_data on the fortran side. I think what you need is an allocation statement somewhere. Assumed size/shape is only relevant when an argument is passed to a subroutine. You have to explicitly declare the size somewhere. So, I think you might need to add

allocate(cn_data(Nbands))

before the call to c_f_pointer. That might fix your pointer problem. Let us know if that helps. It's not clear to me how to use the use_cn_data_struct subroutine. Could you point us to that code as well. Do you actually call into this routine, from fortran or c? do you call it just once to instantiate the pointers on the fortran side and then you use the datatype explicitly wherever you want on the fortran side? is the cn_data fortran type how you access the c side? so cn_data, in general, has to be public and accessible to other parts of the fortran code?

Separately, where does the "21" come from in the vic_cn_data_type declaration. Does that number match something in vic and it won't change. A few thoughts, is it possible to get that number from vic? Maybe we could allocate all those arrays on the fly. I would say that's a longer term implementation issue that should be deferred for now. Short term, how about if you implement a module parameter at the top and set it to 21 and use that instead of hardwiring 21 everywhere. That would look like

integer, parameter :: vsize = 21

and then substitute 21 for vsize. That will make it easier to change later if needed. I have no problem keeping the 21 as is if that's never expected to change. This is a minor detail in either case.

Reply to this email directly or view it on GitHub: https://github.com/jhamman/c-fortran-tests/issues/1#issuecomment-70732864

bartnijssen commented 9 years ago

In Joe's test code all memory allocation is handled in C and he then accesses that memory directly from fortran, so there should be no extra allocation on the fortran side. The c_f_pointer function is what was missing (plus a slight change to the spec of cn_data, i.e. identifying it as a pointer). Apparently it now works as expected. The memory as allocated in C can be directly accessed (and it's value changed) from Fortran.

jhamman commented 9 years ago

@apcraig:

Here is the GNU documentation for the c_f_pointer() subroutine in the ISO_C_BINDING. Using that allows us to translate a C pointer to a Fortran pointer without allocating any new memory.
The call to use_cn_data_struct() is in test1_c.c:L23. Using the ISO_C_BINDING, there is no need for name mangling.

@mabrunke:

Yes, I figured that something like the definition of vic_cn_data_type would be put in a module like clmtype so it could be used elsewhere.
Yes, other structures/types could be added. Hopefully this example makes it clear how you could simplify the interface between VIC and CN. I'll leave that to you.

jhamman commented 9 years ago

@apcraig posed an important question today on the phone. Do the names of the derived types / structures mean anything or is their association based strictly on position in the container? The answer: the association is based only on their position. Cray's reference manual details how this works:

The number and order of the derived type components and C struct members must match. The names of the components are not significant. ... The following example shows how to bind a Fortran variable of derived type to a C function argument of struct type (the name of the derived type is not important in this example because it is bound to a function argument):
/* C struct */
typedef struct
{
  int m, n;
  float r;
} myctype;
USE ISO_C_BINDING
TYPE, BIND(C) :: MYFTYPE
    INTEGER(C_INT) :: I, J
    REAL(C_FLOAT) :: S
END TYPE MYFTYPE

jhamman / c-fortran-tests