Matlab update - Githubissues

KrisThielemans commented 3 years ago

See https://github.com/swig/swig/discussions/2058

[x] take @jaeandersson 's branch and revert https://github.com/jaeandersson/swig/pull/92 as this causes trouble for everyone
[x] merge swig/master up to last commit that @Alzathar merged
[x] do a diff with https://github.com/Alzathar/swig, which should isolate @Alzathar real changes, and create a patch from that
[x] merge swig/master up to current commit
[x] merge https://github.com/KrisThielemans/swig/tree/matlab
[x] merge https://github.com/robotology-dependencies/swig/pull/2
[x] - ~~merge https://github.com/jaeandersson/swig/pull/94 if that change hasn't appeared yet~~ (better to merge this separately, not in this PR)
[x] report on test-suite status
[x] do some fixes

KrisThielemans commented 3 years ago

I haven't kept most of @alzathar's changes to the Examples/matlab. These were moving .cxx files to .c (unnecessary) and renaming the module from swigexample to example (current files use swigexample as copied from the Octave files, but there seems to be no reason to do this, as it can be adapted in the Makefile).

Only changes that I kept are in https://github.com/jaeandersson/swig/pull/96/commits/cb85fe302d5a330d39214ca2928af0bd5f4d87fc

KrisThielemans commented 3 years ago

ok. This is now in better shape than it was.

make -j7 check-matlab-test-suite reports 540 matlab tests passed. There are a few warnings on incorrect MATLAB symbols starting with _ which will need some extra %rename statements, but they don't make the tests fail.
make -j3 check-matlab-examples succesfully runs 3 examples

cd Examples/test-suite/matlab;make check-failing RUNPIPE='>>log'; cat log gives

$ cat test-suite-failed.log 
ERROR: Failed!! sum is 0 but should be 17
li_carrays failed
ERROR: test should have thrown an error
li_cmalloc failed
ERROR: function MyClass_cmethod requires at least 2 arguments
director_basic failed
ERROR: test director ping()
director_unroll failed
ERROR: Unrecognized function or variable 'None'.
overload_complicated failed
ERROR: Failed to use "double" overload!
overload_extend failed
ERROR: in function: CONST_INT1 is  but should be 10
preproc_constants failed
ERROR: Error: File: /cs/research/external/home/kthielem/devel/build/swig/Examples/test-suite/matlab/+smart_pointer_member/Bar.m Line: 17 Column: 26
Invalid text character. Check for unsupported symbol, invisible character, or pasting of non-ASCII characters.
smart_pointer_member failed
ERROR: Undefined function 'swig_typequery' for input arguments of type 'char'.
template_typedef failed
ERROR: Undefined function 'swig_type' for input arguments of type 'template_typedef_cplx.ArithUnaryFunction_double_double'.
template_typedef_cplx failed
ERROR: Undefined function 'swig_type' for input arguments of type 'template_typedef_cplx2.ArithUnaryFunction_double_double'.
template_typedef_cplx2 failed

Some of these are probably easy, a few probably hard.

In any case, @jaeandersson I suggest you merge this now, and we continue from there.

I have followed the above plan, except that I didn't merge #94. As it's a PR here, maybe you can do that as well?

jaeandersson commented 3 years ago

I am not able to run the test suite or the examples using this branch. In MATLAB 2015b on OSX, make check-matlab-examples results in the following error:

checking Examples/matlab/class
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Unknown MEX argument '-I'.
make[3]: *** [matlab_cpp] Error 255
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [class.actionexample] Error 2
checking Examples/matlab/constants
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Unknown MEX argument '-I'.
make[3]: *** [matlab_cpp] Error 255
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [constants.actionexample] Error 2
checking Examples/matlab/funcptr
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Unknown MEX argument '-I'.
make[3]: *** [matlab] Error 255
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [funcptr.actionexample] Error 2
make: *** [check-matlab-examples] Error 2

Using Octave 6.3.0 to run the MATLAB tests (via --enable-octave-for-matlab) doesn't work either:

checking Examples/matlab/class
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
clang: error: no such file or directory: 'example_wrap.o'
make[3]: *** [matlab_cpp] Error 1
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [class.actionexample] Error 2
checking Examples/matlab/constants
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
clang: error: no input files
make[3]: *** [matlab_cpp] Error 1
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [constants.actionexample] Error 2
checking Examples/matlab/funcptr
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
clang: error: no such file or directory: 'example_wrap.o'
make[3]: *** [matlab] Error 1
make[2]: *** [build] Error 2
make[2]: Target `check' not remade because of errors.
make[1]: *** [funcptr.actionexample] Error 2
make: *** [check-matlab-examples] Error 2

Both MATLAB and Octave worked before the PR. @KrisThielemans: Do you have any idea what this is about? Or should I try to pull some PRs separately?

jaeandersson commented 3 years ago

This is the beginning of the output from the test suite before ('make check-matlab-test-suite'):

checking matlab test-suite
checking matlab testcase abstract_access
Building with 'Xcode Clang++'.
MEX completed successfully.

                                                                                                           < M A T L A B (R) >
                                                                                                 Copyright 1984-2015 The MathWorks, Inc.
                                                                                                  R2015b (8.6.0.267246) 64-bit (maci64)
                                                                                                             August 20, 2015

Warning: Duplicate directory name: .
Warning: Duplicate directory name: .

For online documentation, see http://www.mathworks.com/support
For product information, visit www.mathworks.com.

   abstract_access passed
checking matlab testcase abstract_inherit
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_inherit build passed (no runme test present)
checking matlab testcase abstract_inherit_ok
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_inherit_ok build passed (no runme test present)
checking matlab testcase abstract_signature
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_signature build passed (no runme test present)
checking matlab testcase abstract_typedef
Building with 'Xcode Clang++'.
MEX completed successfully.

                                                                                                           < M A T L A B (R) >
                                                                                                 Copyright 1984-2015 The MathWorks, Inc.
                                                                                                  R2015b (8.6.0.267246) 64-bit (maci64)
                                                                                                             August 20, 2015

Warning: Duplicate directory name: .
Warning: Duplicate directory name: .

For online documentation, see http://www.mathworks.com/support
For product information, visit www.mathworks.com.

   abstract_typedef passed
checking matlab testcase abstract_typedef2
Building with 'Xcode Clang++'.
MEX completed successfully.

And after the PR:

jaeandersson@Joels-MacBook-Pro swig % make check-matlab-test-suite
checking matlab test-suite
checking matlab testcase abstract_access
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: syntax error near unexpected token `1,'
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: `exec  "/Applications/MATLAB_R2015b.app/bin/maci64/MATLAB"  -nodisplay -nosplash -batch  try; abstract_access_runme; catch err; fprintf(1, ['ERROR: ' err.message '\\n']); fprintf(2, ['ERROR: ' err.message '\\n']); exit(1); end; exit(0) -nojvm'
   abstract_access failed
make[1]: *** [abstract_access.cpptest] Error 1
checking matlab testcase abstract_inherit
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_inherit build passed (no runme test present)
checking matlab testcase abstract_inherit_ok
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_inherit_ok build passed (no runme test present)
checking matlab testcase abstract_signature
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
   abstract_signature build passed (no runme test present)
checking matlab testcase abstract_typedef
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: syntax error near unexpected token `1,'
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: `exec  "/Applications/MATLAB_R2015b.app/bin/maci64/MATLAB"  -nodisplay -nosplash -batch  try; abstract_typedef_runme; catch err; fprintf(1, ['ERROR: ' err.message '\\n']); fprintf(2, ['ERROR: ' err.message '\\n']); exit(1); end; exit(0) -nojvm'
   abstract_typedef failed
make[1]: *** [abstract_typedef.cpptest] Error 1
checking matlab testcase abstract_typedef2
SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Please read about SWIG experimental languages, http://swig.org/Doc4.0/Introduction.html#Introduction_experimental_status.
Building with 'Xcode Clang++'.
MEX completed successfully.
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: syntax error near unexpected token `1,'
/Applications/MATLAB_R2015b.app/bin/matlab: eval: line 1746: `exec  "/Applications/MATLAB_R2015b.app/bin/maci64/MATLAB"  -nodisplay -nosplash -batch  try; abstract_typedef2_runme; catch err; fprintf(1, ['ERROR: ' err.message '\\n']); fprintf(2, ['ERROR: ' err.message '\\n']); exit(1); end; exit(0) -nojvm'
   abstract_typedef2 failed
make[1]: *** [abstract_typedef2.cpptest] Error 1

jaeandersson commented 3 years ago

Looks like the "batch" option was added in R2019a. Given how much it can cost to upgrade MATLAB, it's probably not good to depend on features that are only available in the latest versions.

jaeandersson commented 3 years ago

Questions/comments:

What MATLAB version are we supporting? I think that should be made clear somewhere.
I'm getting a lot of SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language. Is there a way to suppress this warning?
Changing MATLAB_EXEC back to -r instead of -batch appears to be necessary pre-R2019a. The configure script should probably query the version and pick one or the other
Even after changing "-batch" to "-r", I cannot run the test suite till the end. It ends with template_typedef_import passed make[1]: Target 'check' not remade because of errors. make: *** [check-matlab-test-suite] Error 1

Running the test-suite with Octave appears to mostly work, but gives lots of warnings of the type:

warning: 'uint64 matrix' object indexed with empty index list
warning: called from
subsref at line 29 column 28
Engine at line 9 column 24
Engine at line 12 column 13
abstract_typedef_runme at line 1 column 3

KrisThielemans commented 3 years ago

Looks like the "batch" option was added in R2019a. Given how much it can cost to upgrade MATLAB, it's probably not good to depend on features that are only available in the latest versions.

ok. that's https://github.com/KrisThielemans/swig/commit/448b2df485b19accfa356fd554e2736ba89073d1. I guess we could check with configure somehow if batch is supported?

jaeandersson commented 3 years ago

Update: The test suite - using Octave - eventually stops at:

ERROR: template_typedef_cplx2MEX: Cannot allocate pointer\nERROR: template_typedef_cplx2MEX: Cannot allocate pointer\n
   template_typedef_import failed
make[1]: *** [template_typedef_import.multicpptest] Error 1
make[1]: Target `check' not remade because of errors.

jaeandersson commented 3 years ago

Looks like the "batch" option was added in R2019a. Given how much it can cost to upgrade MATLAB, it's probably not good to depend on features that are only available in the latest versions.

ok. that's KrisThielemans@448b2df. I guess we could check with configure somehow if batch is supported?

The MATLAB command line executable doesn't appear to have a version option. But you can query if "batch" is in the help:

matlab -help | grep -i '\-batch' gives nothing for me
matlab -help | grep -i '\-nodesktop' gives [-desktop | -nodesktop | -nojvm] -nodesktop - Do not start the MATLAB desktop. Use the current

KrisThielemans commented 3 years ago

sigh. some comments:

What MATLAB version are we supporting? I think that should be made clear somewhere.

as we have no descent MATLAB testing facility, we can only claim we ran it with a few versions and list them.
template_typedef_import.multicpptest is the last test that it's supposed to run.
The fact that the MATLAB test says that it didn't run means probably that one of the earlier tests failed but it's hard to see in the junk. You could probably do make check-matlab-test-suite FLAGS= (as FLAGS defaults to -s -k) to see which one it was.
The Octave error seems generated here, but I have no idea if this is a regression or that test always failed for Octave.
warning: 'uint64 matrix' object indexed with empty index list seems to be a problem with the re-introduced subsref implementation, which of course you got rid of... Might be easier to run this code directly in Octave and see what it's complaining about and when.
Matlab version can be obtained from running it, as in the Examples/Makefile. It would have to be run with -r. Simpler seems to be to try and run matlab with -batch and see if it fails. I'd rather do that then relying on a help message. However, my knowledge on configure is very close to 0.
```
* I'm getting a lot of `SWIG:1: Warning 524: Experimental target language. Target language Matlab specified by -matlab is an experimental language`. Is there a way to suppress this warning?
```
Not sure if we'd want to suppress SWIG output. It's terribly annoying of course. Potentially we could use a pipe with grep -v "Warning 524" but not sure if grep is used anywhere else and if @wsfulton would accept using it.

traversaro commented 3 years ago

* as we have no descent MATLAB testing facility

Just FYI as this is rather new feature: on GitHub Action on Linux you can now test against MATLAB, see https://github.com/matlab-actions/overview . However, only MATLAB >= 2020a is available there, so I guess ths is not helpful for checking compatibility with older MATLAB versions.

KrisThielemans commented 3 years ago

amazing! I've merged SWIG/master again such that we could have GHA here. @traversaro want to volunteer adding this? 😄

traversaro commented 3 years ago

@traversaro want to volunteer adding this? 😄

Sure! It probably take some days as I am now back from vacation and processing the backlog, but if you think it make sense we can to open a separate issue so that I avoid to forget, thanks!

KrisThielemans commented 2 years ago

I've fixed some of the above annoyances

running with -r on older matlab (https://github.com/jaeandersson/swig/pull/96/commits/1af02c85082f787dfc189def9864b6a3e4f5f3a7)
disabled the warning SWIG:1: Warning 524: Experimental target language. ... (for MATLAB only) (https://github.com/jaeandersson/swig/pull/96/commits/9538b374869f5554a89a25f8f3f040464d2e09b7)
made the test-suite a bit less noisy https://github.com/jaeandersson/swig/pull/96/commits/63d792e59240f9907f35ae5652715390b291ea2f and https://github.com/jaeandersson/swig/pull/96/commits/14c0caa628ec069a6bb6f29de4a95ee95495de84, fixed #85

I can confirm that on CentOS with MATLAB 2019b, make check-matlab-test-suite works. I'm trying with 2016b but strictly speaking it needs gcc 4.7 which I don't think I can/want to install on my test system .

KrisThielemans commented 2 years ago

I've tried with Octave on Ubuntu 18.04, but struggle. I've tried with both Octave 4.2.2 (Ubuntu package) and self-built 6.4.0. I see the following

make partialcheck-matlab-test-suite works correctly.
The run-time tests all fail as the path isn't set correctly. I need to replace MATLABPATH with OCTAVE_PATH in the Makefile. Then I can proceed. (I see some reports that MATLABPATH is supported by Octave, but it isn't for me. @jaeandersson ?)
I get a ton of warnings like
```
Warning: function ./+li_math/atan.m shadows a built-in function
```
which could be due to https://savannah.gnu.org/bugs/?46849, or could be something else.

With Octave 6.4.0 (but not with 4.2.2) I see warnings like

warning: 'uint64 matrix' object indexed with empty index list
warning: called from
subsref at line 29 column 28

Forging ahead anyway, I get the following run-time errors

unions.ctest and imports.multicpptest fail as Octave doesn't have import yet,

director_default.cpptest`

ERROR: director_defaultMEX: No matching function for overload function 'new_Foo'.  Possible C/C++ prototypes are:
Foo::Foo(mxArray *,int)
Foo::Foo(mxArray *)

'multi_import.multicpptest

multi_import_runme at line 6 column 3
ERROR: multi_import_aMEX: Cannot allocate pointer

template_typedef_import.multicpptest

template_typedef_import_runme at line 6 column 3
ERROR: template_typedef_cplx2MEX: Cannot allocate pointer

@jaeandersson this seems to roughly tally with your experience but not quite. I wonder if your Octave tests didn't fail before this PR because you actually weren't running them (but I didn't check).

KrisThielemans commented 2 years ago

Octave:

The run-time tests all fail as the path isn't set correctly.

I've fixed this with a somewhat ugly work-around in https://github.com/jaeandersson/swig/pull/96/commits/538163e9bd60c2c4e20dce31fa7a7c09bbb8f00f

KrisThielemans commented 2 years ago

I'm trying with 2016b but strictly speaking it needs gcc 4.7 which I don't think I can/want to install on my test system .

This works the same as 2019b, except that I stumble on https://github.com/swig/swig/issues/2127 (as 2016b mex uses gcc -ansi). This is not a a failure of this PR therefore.

KrisThielemans commented 2 years ago

I've reviewed all Octave problems and fixed things related to this PR and created issues for things which are not related to this PR. @jaeandersson I therefore believe that this PR considerably advanced the status and in my opinion should be merged :-)

Here's the detail.

I get a ton of warnings like
Warning: function ./+li_math/atan.m shadows a built-in function
now documented in #101 as unrelated to this PR
With Octave 6.4.0 (but not with 4.2.2) I see warnings like
warning: 'uint64 matrix' object indexed with empty index list
now documented in https://github.com/jaeandersson/swig/issues/99 and fixed in b3594ff3a09b863412d17c647190a67b952cef6a

unions.ctest and imports.multicpptest fail as Octave doesn't have import yet.

It seems that these were overlooked in https://github.com/jaeandersson/swig/commit/b5e75d1891a64000508b511e220483b0547f5428. Now documented in https://github.com/jaeandersson/swig/issues/100 as this is not new to this PR.

director_default.cpptest` failure

This is actually an old one https://github.com/jaeandersson/swig/issues/69 and therefore not related to this PR

multi_import.multicpptest and template_typedef_import.multicpptest fails
...
ERROR: multi_import_aMEX: Cannot allocate pointer

I've created https://github.com/jaeandersson/swig/issues/102. Note that current matlab branch (i.e. before this PR) fails these tests before it ever gets there due to #95

error: subsref: unknown method or property: testx
error: called from
    multi_import_runme at line 7 column 1

KrisThielemans commented 2 years ago

Fixes #95 Fixes #85 Fixes #38 Fixes #9 (@jaeandersson @traversaro please check the new manual!)

KrisThielemans commented 2 years ago

@jaeandersson I think this is as far as I want to go with this PR. Lots of other things to do of course, but no point making this PR even larger than it is already. Please merge (as I cannot).

KrisThielemans commented 2 years ago

Apologies @jaeandersson for pinging again, but if you have no time to merge this, I suggest that we move further development to my fork, or that you give me write permissions here such that I can merge.

jaeandersson commented 2 years ago

Apologies @jaeandersson for pinging again, but if you have no time to merge this, I suggest that we move further development to my fork, or that you give me write permissions here such that I can merge.

It is me who should apologize - I've really struggled to find time for this among my other commitments and I should have recognized that early on and not kept you guys hanging. Lets move the developments to your fork. I don't mind giving you permissions to this fork, but I think it would create confusion to do it in my fork since if I can not reproduce the results which I am not able to do right now. My (commercial) MATLAB license is for R2015b and on OSX, so it's a bit tricky to get everything working. Also using your fork means that you're getting proper credit for the work.

I will try to do some more testing on my end. I'm especially interested in getting Octave to work well. But I can just make a pull request to your branch if there are some minor changes.

jaeandersson / swig

Matlab update #96