swig-fortran / swig

This fork of SWIG creates Fortran wrapper code from C++ headers.
http://www.swig.org
Other
42 stars 11 forks source link

Support 2D Arrays for Fortran #169

Open KeithBallard opened 2 years ago

KeithBallard commented 2 years ago

Hi All,

I have been working on a large C++ project implementing new extended finite element methods. I have been using SWIG for quite a few years to bind my C++ code to Python, but I am quite new to this fork, which is awesome by the way. One thing we often require is passing relatively small dense matrices in and out of functions/methods. Your project provides seamless support for 1D arrays, so I took that as an example and implemented the same integration for 2D arrays. I have tested it for our code, but I am unfamiliar with your test framework, so any help on that front would be appreciated. I look forward to your thoughts and suggestions.

KeithBallard commented 2 years ago

First, I apologize for a very belated response. Over the last month or so, I transitioned from a university position to a scientist at AFRL. I would like to stay engaged with this project and will try to be more responsive in the future.

Thank you for the input and direction. I will take a look at your test-suite for the 1D arrays, and I will add some additional tests for these wrappers later this week.

You bring up an interesting point. Honestly, I didn't realize ARRAY[][] had to be C/C++ syntax. SWIG did not complain about when I tested it in our code. I am actually using today.

Regarding your last point, yeah...I am assuming contiguity of the memory. I don't know a straight forward way to check if that assumption is satisfied, but I honestly see it as something that if documented in the API clearly is up to the developer? What do you think? However...padding is something I worry about for aligned 2D arrays. Perhaps I can check for that in the wrapper. I will think about it some more.

sethrj commented 2 years ago

First, I apologize for a very belated response. Over the last month or so, I transitioned from a university position to a scientist at AFRL. I would like to stay engaged with this project and will try to be more responsive in the future.

Congratulations! No worries on the response: I myself didn't see your MR until over a month past its creation.

You bring up an interesting point. Honestly, I didn't realize ARRAY[][] had to be C/C++ syntax. SWIG did not complain about when I tested it in our code. I am actually using today.

Yes, SWIG's parser and internal processing aren't very standards-compliant. I also had tried something like ARRAY[][] earlier in the project and got some pointed feedback from the SWIG maintainer

Regarding your last point, yeah...I am assuming contiguity of the memory. I don't know a straight forward way to check if that assumption is satisfied, but I honestly see it as something that if documented in the API clearly is up to the developer? What do you think? However...padding is something I worry about for aligned 2D arrays. Perhaps I can check for that in the wrapper. I will think about it some more.

I think you could write a templated (or SWIG "templated" fragment) C++ function

template<class T>
bool is_contiguous(int nr, int nc, T* r1c1, T* r1c2, T* r2c1)
{
   if (nc > 1 && r1c2 != r1c1 + nr) return false;
   if (nr > 1 && r2c1 != r1c1 + nc) return false;
  return true;
}

and add corresponding logic to call from the fortran side.

Of course, we could also wait for F2008's CONTIGUOUS attribute...

KeithBallard commented 2 years ago

Note: I just rebased on master to incorporate the variable name change.

KeithBallard commented 2 years ago

I think you could write a templated (or SWIG "templated" fragment) C++ function

template<class T>
bool is_contiguous(int nr, int nc, T* r1c1, T* r1c2, T* r2c1)
{
   if (nc > 1 && r1c2 != r1c1 + nr) return false;
   if (nr > 1 && r2c1 != r1c1 + nc) return false;
  return true;
}

and add corresponding logic to call from the fortran side.

Of course, we could also wait for F2008's CONTIGUOUS attribute...

I like the idea, though I could see the test failing for 2D arrays with 1-row, 1-column, or a single value. I suppose there can be checks for these special cases, but it does add further complexity. A CONTIGUOUS attribute would be the most ideal. It appears supported by my compiler (Intel Fotran 2022), but is SWIG-FORTRAN based tied to the 2003 standard?

KeithBallard commented 2 years ago

It looks like the only test I have for %fortran_array_pointer is in Examples/fortran/thinvec, but there are other array mapping tests in Examples/test-suite/fortran/fortran_array_typemap_runme.f90 (using Examples/test-suite/fortran_array_typemap.i) so could you cook something up for the new ARRAY[][] typemap for that as well? If you have a local SWIG build, to test you just need to cd $BUILD/Examples/test-suite/fortran && make fortran_array_typemap.cpptest.

I am developing on Windows, and I could not figure out how to build that test-suite. I ran the SWIG project tests, but that only tested the binary. Do you have any experience building them for Windows?

However, I did add to your test to cover the new functionality (though I did not test it myself).

sethrj commented 2 years ago

@KeithBallard I haven't developed on Windows since I was a student worker way back in college, so I can't help you out there. Since you're in the scientific computing field I would imagine using a linux environment via WSL would be a good option in general.

As it stands, the test is failing, among other things a missing <stdint.h> and some type conflicts:

2022-07-05T09:31:06.4775654Z fortran_array_typemap_wrap.c:310:38: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4776496Z   310 | void set_values_int(int* data, const uint64_t rows, const uint64_t cols, int value) {
2022-07-05T09:31:06.4777058Z       |                                      ^~~~~~~~
2022-07-05T09:31:06.4777632Z fortran_array_typemap_wrap.c:310:59: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4778177Z   310 | void set_values_int(int* data, const uint64_t rows, const uint64_t cols, int value) {
2022-07-05T09:31:06.4831964Z       |                                                           ^~~~~~~~
2022-07-05T09:31:06.4832652Z fortran_array_typemap_wrap.c:310:6: error: conflicting types for ‘set_values_int’
2022-07-05T09:31:06.4833056Z   310 | void set_values_int(int* data, const uint64_t rows, const uint64_t cols, int value) {
2022-07-05T09:31:06.4833349Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4833802Z fortran_array_typemap_wrap.c:194:6: note: previous definition of ‘set_values_int’ was here
2022-07-05T09:31:06.4845052Z   194 | void set_values_int(int *DATA, size_t SIZE, int value) {
2022-07-05T09:31:06.4845341Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4845823Z fortran_array_typemap_wrap.c:317:41: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4846211Z   317 | void set_values_dbl(double* data, const uint64_t rows, const uint64_t cols, double value) {
2022-07-05T09:31:06.4846538Z       |                                         ^~~~~~~~
2022-07-05T09:31:06.4846921Z fortran_array_typemap_wrap.c:317:62: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4847568Z   317 | void set_values_dbl(double* data, const uint64_t rows, const uint64_t cols, double value) {
2022-07-05T09:31:06.4847901Z       |                                                              ^~~~~~~~
2022-07-05T09:31:06.4848341Z fortran_array_typemap_wrap.c:317:6: error: conflicting types for ‘set_values_dbl’
2022-07-05T09:31:06.4848741Z   317 | void set_values_dbl(double* data, const uint64_t rows, const uint64_t cols, double value) {
2022-07-05T09:31:06.4849043Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4849451Z fortran_array_typemap_wrap.c:202:6: note: previous definition of ‘set_values_dbl’ was here
2022-07-05T09:31:06.4849821Z   202 | void set_values_dbl(double *DATA, int SIZE, double value) {
2022-07-05T09:31:06.4850085Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4850469Z fortran_array_typemap_wrap.c:324:34: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4850825Z   324 | int accum(const int* data, const uint64_t rows, const uint64_t cols) {
2022-07-05T09:31:06.4851117Z       |                                  ^~~~~~~~
2022-07-05T09:31:06.4851493Z fortran_array_typemap_wrap.c:324:55: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4851840Z   324 | int accum(const int* data, const uint64_t rows, const uint64_t cols) {
2022-07-05T09:31:06.4852145Z       |                                                       ^~~~~~~~
2022-07-05T09:31:06.4852537Z fortran_array_typemap_wrap.c:324:5: error: conflicting types for ‘accum’
2022-07-05T09:31:06.4852889Z   324 | int accum(const int* data, const uint64_t rows, const uint64_t cols) {
2022-07-05T09:31:06.4853150Z       |     ^~~~~
2022-07-05T09:31:06.4853531Z fortran_array_typemap_wrap.c:210:5: note: previous definition of ‘accum’ was here
2022-07-05T09:31:06.4853887Z   210 | int accum(const int *DATA, size_t SIZE) {
2022-07-05T09:31:06.4854117Z       |     ^~~~~
2022-07-05T09:31:06.4854438Z fortran_array_typemap_wrap.c: In function ‘accum’:
2022-07-05T09:31:06.4855032Z fortran_array_typemap_wrap.c:326:14: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
2022-07-05T09:31:06.4855430Z   326 |   int* end = data + rows * cols;
2022-07-05T09:31:06.4855659Z       |              ^~~~
2022-07-05T09:31:06.4856040Z fortran_array_typemap_wrap.c: In function ‘_wrap_set_values_int__SWIG_0’:
2022-07-05T09:31:06.4856527Z fortran_array_typemap_wrap.c:342:3: error: too few arguments to function ‘set_values_int’
2022-07-05T09:31:06.4856854Z   342 |   set_values_int(arg1,arg2,arg3);
2022-07-05T09:31:06.4857083Z       |   ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4857332Z fortran_array_typemap_wrap.c:310:6: note: declared here
2022-07-05T09:31:06.4857679Z   310 | void set_values_int(int* data, const uint64_t rows, const uint64_t cols, int value) {
2022-07-05T09:31:06.4857968Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4858346Z fortran_array_typemap_wrap.c: In function ‘_wrap_set_values_dbl__SWIG_0’:
2022-07-05T09:31:06.4858832Z fortran_array_typemap_wrap.c:354:3: error: too few arguments to function ‘set_values_dbl’
2022-07-05T09:31:06.4859165Z   354 |   set_values_dbl(arg1,arg2,arg3);
2022-07-05T09:31:06.4859389Z       |   ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4859653Z fortran_array_typemap_wrap.c:317:6: note: declared here
2022-07-05T09:31:06.4860010Z   317 | void set_values_dbl(double* data, const uint64_t rows, const uint64_t cols, double value) {
2022-07-05T09:31:06.4860305Z       |      ^~~~~~~~~~~~~~
2022-07-05T09:31:06.4860672Z fortran_array_typemap_wrap.c: In function ‘_wrap_accum__SWIG_0’:
2022-07-05T09:31:06.4861121Z fortran_array_typemap_wrap.c:366:17: error: too few arguments to function ‘accum’
2022-07-05T09:31:06.4861455Z   366 |   result = (int)accum((int const *)arg1,arg2);
2022-07-05T09:31:06.4861696Z       |                 ^~~~~
2022-07-05T09:31:06.4861959Z fortran_array_typemap_wrap.c:324:5: note: declared here
2022-07-05T09:31:06.4862290Z   324 | int accum(const int* data, const uint64_t rows, const uint64_t cols) {
2022-07-05T09:31:06.4862622Z       |     ^~~~~
2022-07-05T09:31:06.4862985Z fortran_array_typemap_wrap.c: In function ‘_wrap_set_values_int__SWIG_1’:
2022-07-05T09:31:06.4863488Z fortran_array_typemap_wrap.c:448:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4863779Z   448 |   uint64_t arg2 ;
2022-07-05T09:31:06.4863985Z       |   ^~~~~~~~
2022-07-05T09:31:06.4864355Z fortran_array_typemap_wrap.c:449:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4864621Z   449 |   uint64_t arg3 ;
2022-07-05T09:31:06.4864825Z       |   ^~~~~~~~
2022-07-05T09:31:06.4865248Z fortran_array_typemap_wrap.c:454:13: error: ‘uint64_t’ undeclared (first use in this function)
2022-07-05T09:31:06.4865656Z   454 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4865893Z       |             ^~~~~~~~
2022-07-05T09:31:06.4866254Z fortran_array_typemap_wrap.c:454:13: note: each undeclared identifier is reported only once for each function it appears in
2022-07-05T09:31:06.4866770Z fortran_array_typemap_wrap.c:454:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4867163Z   454 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4867406Z       |                       ^
2022-07-05T09:31:06.4867797Z fortran_array_typemap_wrap.c:456:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4868178Z   456 |   arg3 = *((uint64_t *)(farg3->cptr));
2022-07-05T09:31:06.4868396Z       |                       ^
2022-07-05T09:31:06.4868777Z fortran_array_typemap_wrap.c: In function ‘_wrap_set_values_dbl__SWIG_1’:
2022-07-05T09:31:06.4869223Z fortran_array_typemap_wrap.c:464:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4869502Z   464 |   uint64_t arg2 ;
2022-07-05T09:31:06.4869708Z       |   ^~~~~~~~
2022-07-05T09:31:06.4870075Z fortran_array_typemap_wrap.c:465:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4870341Z   465 |   uint64_t arg3 ;
2022-07-05T09:31:06.4870545Z       |   ^~~~~~~~
2022-07-05T09:31:06.4870958Z fortran_array_typemap_wrap.c:470:13: error: ‘uint64_t’ undeclared (first use in this function)
2022-07-05T09:31:06.4871363Z   470 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4871599Z       |             ^~~~~~~~
2022-07-05T09:31:06.4871980Z fortran_array_typemap_wrap.c:470:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4872367Z   470 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4872598Z       |                       ^
2022-07-05T09:31:06.4872990Z fortran_array_typemap_wrap.c:472:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4873366Z   472 |   arg3 = *((uint64_t *)(farg3->cptr));
2022-07-05T09:31:06.4873595Z       |                       ^
2022-07-05T09:31:06.4873940Z fortran_array_typemap_wrap.c: In function ‘_wrap_accum__SWIG_1’:
2022-07-05T09:31:06.4874382Z fortran_array_typemap_wrap.c:481:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4874660Z   481 |   uint64_t arg2 ;
2022-07-05T09:31:06.4874862Z       |   ^~~~~~~~
2022-07-05T09:31:06.4875225Z fortran_array_typemap_wrap.c:482:3: error: unknown type name ‘uint64_t’
2022-07-05T09:31:06.4875495Z   482 |   uint64_t arg3 ;
2022-07-05T09:31:06.4875697Z       |   ^~~~~~~~
2022-07-05T09:31:06.4876114Z fortran_array_typemap_wrap.c:487:13: error: ‘uint64_t’ undeclared (first use in this function)
2022-07-05T09:31:06.4876512Z   487 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4876742Z       |             ^~~~~~~~
2022-07-05T09:31:06.4877131Z fortran_array_typemap_wrap.c:487:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4877501Z   487 |   arg2 = *((uint64_t *)(farg2->cptr));
2022-07-05T09:31:06.4877731Z       |                       ^
2022-07-05T09:31:06.4878124Z fortran_array_typemap_wrap.c:489:23: error: expected expression before ‘)’ token
2022-07-05T09:31:06.4878500Z   489 |   arg3 = *((uint64_t *)(farg3->cptr));
2022-07-05T09:31:06.4878729Z       |                       ^
2022-07-05T09:31:06.4878957Z cc1: all warnings being treated as errors
2022-07-05T09:31:06.4879244Z make[2]: *** [../../../Examples/Makefile:1658: fortran] Error 1
2022-07-05T09:31:06.4879629Z make[1]: *** [Makefile:134: fortran_array_typemap.ctest] Error 2
KeithBallard commented 2 years ago

@KeithBallard I haven't developed on Windows since I was a student worker way back in college, so I can't help you out there. Since you're in the scientific computing field I would imagine using a linux environment via WSL would be a good option in general.

No worries. Since just transferring jobs, I have a lot of work ahead of me to set up various dev environments. I haven't set up my Linux environment yet. I bounce between Linux and Windows since our software has to be used on both systems. I will sort out the errors after I set up Linux, though I wish the test suite could be built on Windows. Perhaps I will try to get both working.