Prior to this patch, the itmask array held elements of the logical dtype. This patch modified the Fortran layer so that the variable always has a dtype called MASK_TYPE. In reality, MASK_TYPE is a macro that expands to integer*4 (the integer-size was an arbitrary choice).
Motivation
As I previously shared during a development meeting, the usage of the logical dtype is a bit of an issue for some Fortran->C transcription efforts. The issue is that:
the Fortran specification doesn't define the representation for .true. and .false. values. Consequently, different implementations make different choices (as I understand it, gfortran and ifort originally made different choices -- ifort only changed their mind in the last 10 years).
importantly, this means that there isn't a clear conversion from Fortan's logical dtype and C's dtype. A crude workaround for this is to convert from logical to int before a Fortran function calls a C function.
This workaround is fine if you are converting a lone scalar logical argument (this is done in #160).
However, it isn't ideal when you have to pass a logical array, like itmask, to a C function. You ultimately need to allocate a temporary intermediate array and pass it through to the C function (as is done by #153). This is bad for performance since heap memory needs to be freshly allocated/deallocated off the heap during each function call. [^1]
By now storing integers within itmask (as is done by this PR), any transcribed C function now knows how to directly interpret the contents of the array.
Alternative
The alternative to this patch would to declare itmask to be a variable with the type logical(kind=c_bool), where c_bool would be provided by the ISO_C_BINDING Fortran module (formally, the module was introduced with Fortran 2003, but all major fortran compilers support it in other modes). Essentially, this would tell the Fortran compiler to represent it in the same way as the boolean dtype introduced to C in 1999 (it was formally called _Bool, but macros were provided so that you could refer to it as bool).
This alternative would have been less invasive. However, there could be issues if we ever decided to compile Grackle with a C++ compiler. These issues could arise because C's _Bool dtype may not have the same representation as C++'s bool dtype (I think this is mostly an issue when you have compilers provided by different vendors). In order to support GPUs, Grackle will probably need to start using C++ compilers (even if we just restrict to the common subset of the language that is shared with C).
At a previous development meeting, it was decided that the chosen solution implemented by this PR is preferred.
To Summarize
This patch will allow us to simplify the proposed Fortran->C conversion in gh-#153 (and it will removed performance-overhead of the extra heap-allocations currently required in the absence of this patch).
In the same way, this patch will significantly simplify the process of converting converting any Fortran functionality that takes an iteration mask and operates on a slice. This is the vast majority of all grackle functionality! [^2]
[^1]: Let me be more concrete. In the absence of this patch, #153 currently introduces "stub" functions for each transcribed function. Each "stub" function takes all of the arguments for the transcribed function. The "stub" function's only purpose is to forward all of the arguments, other than itmask, directly onto the transcribed function. Instead of passing itmask, the "stub" function instead passes through a temporary array that it filled with coerced values from itmask. Once this patch is merged, we can get rid of the "stub" functions and pass itmask directly to the transcribed function.
[^2]: Aside: #160 shows a notable example where this patch isn't particularly relevant, since that PR converts a set of functions that only operate on a single value rather than on an array of values. But these are very much an exception to the rule. (At this time, I don't think there are any other functions in the fortran layer that don't operate on an array of values).
Description
Prior to this patch, the
itmaskarray held elements of thelogicaldtype. This patch modified the Fortran layer so that the variable always has a dtype calledMASK_TYPE. In reality,MASK_TYPEis a macro that expands tointeger*4(the integer-size was an arbitrary choice).Motivation
As I previously shared during a development meeting, the usage of the
logicaldtype is a bit of an issue for some Fortran->C transcription efforts. The issue is that:the Fortran specification doesn't define the representation for
.true.and.false.values. Consequently, different implementations make different choices (as I understand it, gfortran and ifort originally made different choices -- ifort only changed their mind in the last 10 years).importantly, this means that there isn't a clear conversion from Fortan's
logicaldtype and C's dtype. A crude workaround for this is to convert fromlogicaltointbefore a Fortran function calls a C function.This workaround is fine if you are converting a lone scalar
logicalargument (this is done in #160).However, it isn't ideal when you have to pass a
logicalarray, likeitmask, to a C function. You ultimately need to allocate a temporary intermediate array and pass it through to the C function (as is done by #153). This is bad for performance since heap memory needs to be freshly allocated/deallocated off the heap during each function call. [^1]By now storing integers within
itmask(as is done by this PR), any transcribed C function now knows how to directly interpret the contents of the array.Alternative
The alternative to this patch would to declare
itmaskto be a variable with the typelogical(kind=c_bool), wherec_boolwould be provided by theISO_C_BINDINGFortran module (formally, the module was introduced with Fortran 2003, but all major fortran compilers support it in other modes). Essentially, this would tell the Fortran compiler to represent it in the same way as the boolean dtype introduced to C in 1999 (it was formally called_Bool, but macros were provided so that you could refer to it asbool).This alternative would have been less invasive. However, there could be issues if we ever decided to compile Grackle with a C++ compiler. These issues could arise because C's
_Booldtype may not have the same representation as C++'sbooldtype (I think this is mostly an issue when you have compilers provided by different vendors). In order to support GPUs, Grackle will probably need to start using C++ compilers (even if we just restrict to the common subset of the language that is shared with C).At a previous development meeting, it was decided that the chosen solution implemented by this PR is preferred.
To Summarize
This patch will allow us to simplify the proposed Fortran->C conversion in gh-#153 (and it will removed performance-overhead of the extra heap-allocations currently required in the absence of this patch).
In the same way, this patch will significantly simplify the process of converting converting any Fortran functionality that takes an iteration mask and operates on a slice. This is the vast majority of all grackle functionality! [^2]
[^1]: Let me be more concrete. In the absence of this patch, #153 currently introduces "stub" functions for each transcribed function. Each "stub" function takes all of the arguments for the transcribed function. The "stub" function's only purpose is to forward all of the arguments, other than
itmask, directly onto the transcribed function. Instead of passingitmask, the "stub" function instead passes through a temporary array that it filled with coerced values fromitmask. Once this patch is merged, we can get rid of the "stub" functions and passitmaskdirectly to the transcribed function.[^2]: Aside: #160 shows a notable example where this patch isn't particularly relevant, since that PR converts a set of functions that only operate on a single value rather than on an array of values. But these are very much an exception to the rule. (At this time, I don't think there are any other functions in the fortran layer that don't operate on an array of values).