Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for FAST_SFunc using Matlab action to integrate with CI #1703

Merged
merged 26 commits into from
Aug 3, 2023

Conversation

reos-rcrozier
Copy link
Contributor

@reos-rcrozier reos-rcrozier commented Jul 26, 2023

Feature or improvement description
This change introduces a matlab test case and a github workflow for testing the build and running of the Simulink S-function. The test currently fails, but this is because of a real problem of undefined symbols, hence the need for the test.

Related issue, if one exists

Impacted areas of the software
Github CI tests

Additional supporting information
Currently attempting to run the built S-function on Linux results in the following error

Invalid MEX-file '/home/rcrozier/src/openfast-reos-git/build/glue-codes/simulink/FAST_SFunc.mexa64':
        /home/rcrozier/src/openfast-reos-git/build/glue-codes/simulink/FAST_SFunc.mexa64: undefined symbol:
        __polynomialroots_MOD_quarticroots

This is exhibited in the test.

Test results, if applicable

Running test_openfast_simulink

================================================================================
Error occurred in test_openfast_simulink/testOpenLoopRuns and it did not run to completion.
    ---------
    Error ID:
    ---------
    'Simulink:SFunctions:MexSFcnSizesInitErr'
    --------------
    Error Details:
    --------------
    Error using test_openfast_simulink/testOpenLoopRuns
    Error while obtaining sizes from MEX S-function 'FAST_SFunc' in 'OpenLoop/FAST Nonlinear Wind
    Turbine/S-Function'.
    
    Caused by:
        Error using test_openfast_simulink/testOpenLoopRuns
        Invalid MEX-file '/home/rcrozier/src/openfast-reos-git/build/glue-codes/simulink/FAST_SFunc.mexa64':
        /home/rcrozier/src/openfast-reos-git/build/glue-codes/simulink/FAST_SFunc.mexa64: undefined symbol:
        __polynomialroots_MOD_quarticroots
================================================================================
.
Done test_openfast_simulink
__________

Failure Summary:

     Name                                     Failed  Incomplete  Reason(s)
    ========================================================================
     test_openfast_simulink/testOpenLoopRuns    X         X       Errored.

This branch is to modify the workflow to add tests for the build of the Matlab Simulink interface.
Unit test for Matlab Simulink interface.
Use environment variable to get workspace root
Added Simulink test run
Rename simulink test and add step to set up Matlab.
Change class name to match file name.
Ensure variables are available to Simulink model
Actually build FAST_SFunc before testing
@reos-rcrozier
Copy link
Contributor Author

Tests are failing before my new test can even be run, failures definitely seem completely unrelated to my new test.

@reos-rcrozier
Copy link
Contributor Author

For convenience, here is the main error output, seems like there's a problem with missing targets related to openfoam:

CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamtypeslib>

  No target "foamtypeslib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamfastlib>

  No target "foamfastlib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamtypeslib>

  No target "foamtypeslib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamfastlib>

  No target "foamfastlib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamtypeslib>

  No target "foamtypeslib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1[170](https://github.com/OpenFAST/openfast/actions/runs/5669163073/job/15361294618?pr=1703#step:7:171) (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamfastlib>

  No target "foamfastlib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamtypeslib>

  No target "foamtypeslib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamfastlib>

  No target "foamfastlib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamtypeslib>

  No target "foamtypeslib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


CMake Error at /usr/local/share/cmake-3.27/Modules/FindMatlab.cmake:1170 (target_link_libraries):
  Error evaluating generator expression:

    $<TARGET_FILE:foamfastlib>

  No target "foamfastlib"
Call Stack (most recent call first):
  glue-codes/simulink/CMakeLists.txt:23 (matlab_add_mex)


-- Generating done (0.7s)
CMake Generate step failed.  Build files cannot be regenerated correctly.
Error: Process completed with exit code 1.

Remove references to openfoam library as that module has been removed
Add references to external inflow module libs
update another reference from openfoam to  externalinflow
@reos-rcrozier
Copy link
Contributor Author

I resolved the issue with openfoam --> extinflow so now the only test failure is unrelated to the PR changes (just exposing a previously hidden build failure). I would like to request a review please @andrew-platt @deslaughter

Add missing seastlib
@bjonkman
Copy link
Contributor

@deslaughter and @andrew-platt: perhaps we should merge #1682 with its fixes for Simulink into dev as well as 3.5.1 (or just merge all of 3.5.1 into dev now so we can address merge conflicts earlier)? That might help solve this issue, too.

@andrew-platt andrew-platt added this to the v4.0.0 milestone Jul 27, 2023
@andrew-platt
Copy link
Collaborator

@reos-rcrozier, thanks for putting together this test case! This will definitely help when we make changes to the build system and don't remember to propagate them all the way (this was an oversight on my part during the name change).

@bjonkman, I agree on merging 3.5.1 to dev now. There are some fixes there that would be good to get into dev sooner (we'll continue with getting 3.5.1 actually released, then merge 3.5.1/main to dev again at that point).

Copy link
Contributor

@deslaughter deslaughter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reos-rcrozier Thanks for setting this up and working out the bugs. As @bjonkman mentioned, there was a PR to rc-3.5.1 that should fix the issue with the missing symbols. I'm getting together a PR merging rc-3.5.1 into dev. Once that is merged you can pull in the changes and see if it fixes the symbol issue.

@reos-rcrozier
Copy link
Contributor Author

Thanks, sounds good

@reos-rcrozier
Copy link
Contributor Author

Regarding rc-3.5.1 I checked out that branch and built it. When I do so, and run the RunOpenLoop.m script, instead of throwing an error about undefined symbols Matlab (R2023a in this case) simply immediately crashes.

@reos-rcrozier
Copy link
Contributor Author

If I build rc-3.5.1 with USE_LOCAL_STATIC_LAPACK=ON it runs fine. Might need to update test to do this, but lets see.

@deslaughter
Copy link
Contributor

Could you try it with -DBLA_STATIC=ON? That should force it to statically link the system LAPACK library.

@reos-rcrozier
Copy link
Contributor Author

reos-rcrozier commented Jul 28, 2023

with

cmake -LH ~/src/openfast-official-git -DBUILD_FASTFARM=ON -DBUILD_OPENFAST_CPP_API=ON -DBUILD_OPENFAST_SIMULINK_API=ON -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=/usr/local -DDOUBLE_PRECISION=ON -DOPENMP=OFF -DUSE_LOCAL_STATIC_LAPACK=OFF -DBLA_STATIC=ON
I get


-- Could NOT find LAPACK (missing: LAPACK_LIBRARIES) 
CMake Error at CMakeLists.txt:155 (message):
  Unable to find BLAS and LAPACK

I have a libblas.a in /usr/lib/x86_64-linux-gnu/libblas.a

@reos-rcrozier
Copy link
Contributor Author

@deslaughter
Copy link
Contributor

I agree that the issue you linked is causing the problem. It said that LAPACK isn't found when BLA_VENDOR isn't set or is set to All. Could you try running the configuration with -DBLA_VENDOR=Generic and -DBLA_STATIC=ON? If that doesn't work then I think you should add -DUSE_LOCAL_STATIC_LAPACK=ON to the Github Action.

@reos-rcrozier
Copy link
Contributor Author

Adding -DBLA_VENDOR=Generic (in addition to -DBLA_STATIC=ON) to the cmake command allowed it to successfully build and run for me.

@deslaughter
Copy link
Contributor

@reos-rcrozier I have merged branch rc-3.5.1 into dev so the fixes to the Simulink build should be there. Please update your PR and I'll get it merged in. Also, I added OpenBLAS to the Github Actions as it improved the speed of some regression tests, so BLA_VENDOR=OpenBLAS is already set. If you could add -DBLA_STATIC=1 to the relevant build commands, I would appreciate it. Thanks for working on this.

replace references to foamfastlib with extinflowlib again
Add missing openblas packages for simulink test
again added missing seastatelib
Add BLA_STATIC=ON to simulink test build
Try using local static lapack for simulink
@reos-rcrozier
Copy link
Contributor Author

@deslaughter I tried building with -DBLA_STATIC:BOOL=ON, but this gives the following:

   ================================================================================
  Error occurred in test_openfast_simulink/testOpenLoopRuns and it did not run to completion.
      ---------
      Error ID:
      ---------
      'Simulink:SFunctions:SFcnErrorStatus'
      --------------
      Error Details:
      --------------
      Error using test_openfast_simulink/testOpenLoopRuns
      Error reported by S-function 'FAST_SFunc' in 'OpenLoop/FAST Nonlinear Wind
      Turbine/S-Function':
      FAST_Solution0:CalcOutputs_And_SolveForInputs:SolveOption2:SolveOption2a_Inp2BD:ED_CalcContStateDeriv:LAPACK_DGETRF:
      illegal value in argument           -4.
  ================================================================================

I'm trying again with -DUSE_LOCAL_STATIC_LAPACK="ON" to see if that works.

@reos-rcrozier
Copy link
Contributor Author

Successful with -DUSE_LOCAL_STATIC_LAPACK="ON". I have seen the error illegal value in argument -4. in the past. It may even be what prompted me to introduce this in the first place.

@deslaughter
Copy link
Contributor

I'm fine with merging this using -DUSE_LOCAL_STATIC_LAPACK="ON" as it tests this additional functionality and doesn't affect any of the other tests. I may try to get it working with OpenBLAS in the future, but getting this test in now will be very helpful. Thanks for putting this together!

@deslaughter deslaughter merged commit 10c9e33 into OpenFAST:dev Aug 3, 2023
20 checks passed
@reos-rcrozier
Copy link
Contributor Author

Yes, I'm not sure why it didn't work, I may not have set it up correctly. Glad to have this merged, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants