mdolab / CMPLXFOIL

GNU General Public License v2.0
29 stars 20 forks source link

Added option to set nCrit #29

Closed sseraj closed 1 month ago

sseraj commented 1 month ago

Purpose

I added an option to set nCrit. This required setting the acrit and lvconv variables in Fortran.

Expected time until merged

1 week

Type of change

Testing

I added a test similar to the trip location test.

Checklist

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 66.31%. Comparing base (581dcfd) to head (7652c3d).

Files Patch % Lines
cmplxfoil/__init__.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #29 +/- ## ========================================== + Coverage 66.19% 66.31% +0.11% ========================================== Files 4 4 Lines 565 567 +2 ========================================== + Hits 374 376 +2 Misses 191 191 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sseraj commented 1 month ago

I'm not exactly sure what effect setting LVCONV to False has on the initialization. I was following what the GUI does when setting nCrit: https://github.com/mdolab/CMPLXFOIL/blob/581dcfddfac2522e94b0b86840c8c46218b04850/src/xoper.f#L599-L605

You're right that it doesn't matter right now because it's only set at the beginning. Changing nCrit (or other Fortran options) between analyses should be possible, but doing this in a user-friendly way would require implementing a setOption function like in ADflow that changes the required Fortran variables.

sseraj commented 1 month ago

I could be wrong, but I think the setOption method defined in the BaseSolver class that CMPLXFOIL inherits is sufficient.

You're right, the Fortran variables are set in __call__, so changing nCrit between runs should already work.

Also, I confirmed locally that if you call the solver twice it starts from the previous solution, even when LVCONV is set to False.

Thanks for checking the initialization!