Remove local complexify and use shared library and scripts

eirikurj commented 1 year ago

Purpose

This PR removes the local complexify script and module and uses the shared complexify library. Docs are also updated to reflect these changes.

Expected time until merged

1 week

Type of change

[ ] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (non-backwards-compatible fix or feature)
[ ] Code style update (formatting, renaming)
[x] Refactoring (no functional changes, no API changes)
[ ] Documentation update
[x] Maintenance update
[ ] Other (please describe)

Testing

Checklist

[ ] I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
[ ] I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
[ ] I have run unit and regression tests which pass locally with my changes
[ ] I have added new tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation

codecov[bot] commented 1 year ago

Codecov Report

Merging #82 (d0e58bf) into main (dda9b29) will increase coverage by 0.07%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
+ Coverage   73.43%   73.50%   +0.07%     
==========================================
  Files           6        6              
  Lines         753      755       +2     
==========================================
+ Hits          553      555       +2     
  Misses        200      200

see 2 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

eirikurj commented 1 year ago

The Tapenade issues are to be due to a recent update/change in Tapenade behavior. Not sure why they changed this, but now they are appending *4 to an integer declaration. Compiling this code will fail with something like,

   29 |    INTEGER*4 :: branch
      |            1
Error: GNU Extension: Nonstandard type declaration INTEGER*4 at (1)

This is because we are requesting to follow a specific standard (2008 in this case) where this is not allowed. Dropping -std=f2008 would compile the code. I already encountered this as part of supporting Tapenade 3.16 for ADflow. There I opted for searching and removing these in the Tapenade generated code. A better approach would be to define a kind for these variables, and this should probably be done directly at the Tapenade level. However, I am not sure if they will support this as they only claim to support 95, with limited support for newer standards.

sseraj commented 1 year ago

I am confused why Tapenade made this breaking change and why it was seemingly backported to the 3.16 distrib tarball that we download in our Tapenade check. Ideally, we would be able to pin a static version of Tapenade, but I don't see a publicly accessible archive of previous distributions on their GitLab page. I still have the 3.16 tarball that works on my computer, so we could self-host like we do for 3.10.

eirikurj commented 1 year ago

Yes, I have to agree, not sure what the motivation was. They seem to generate a binary release directly from their develop branch and post online. Ideally, we would like to use the latest and know as soon as there are changes, but as you said maybe its better here to just use a fixed version of Tapenade. They have all their releases here https://gitlab.inria.fr/tapenade/tapenade/-/packages/894, so perhaps we could just download the desired release directly from there.

sseraj commented 1 year ago

They have all their releases here https://gitlab.inria.fr/tapenade/tapenade/-/packages/894, so perhaps we could just download the desired release directly from there.

Nice find! It looks like they only have releases from the develop branch, but these links are probably more stable than the link we currently use.

eirikurj commented 1 year ago

Created a PR for a tapenade fix here https://github.com/mdolab/.github/pull/58. Hopefully this will address the issue. Will update files once merged.

mdolab / idwarp