-
Notifications
You must be signed in to change notification settings - Fork 507
Performance improvements in dev-tc #3125
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
base: dev-tc
Are you sure you want to change the base?
Conversation
… load calculation. Hydrodyn doesn't need hydrostatic loads when calculating dYdu and perturbing velocities/accelerations. Resulted in 20% speedup.
…rID_None when entering the function. Therefore, each function doesn't need to set ErrMsg = "" at the beginning which reduces some overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements performance optimizations across multiple modules to reduce computation time in HydroDyn, Morison, NWTC_Num, and NWTC_Base. The primary improvement is adding an optional flag to skip expensive hydrostatic load calculations during Jacobian calculations when perturbing velocity/acceleration inputs.
Key Changes:
- Added optional
calcHstLdsparameter to HydroDyn_CalcOutput and Morison_CalcOutput to conditionally skip hydrostatic load calculations - Removed unused error handling parameters from DCM_LogMap functions throughout the codebase
- Eliminated redundant
ErrMsg = ""initializations by relying on SetErrStat's conditional behavior - Refactored Morison hydrostatic load calculations from recursive to iterative stack-based approach
- Optimized OLAF/FVW functions by moving variables out of loops and reducing array access overhead
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/hydrodyn/src/HydroDyn.f90 | Added optional calcMorisonHstLds parameter to HydroDyn_CalcOutput and HD_JacobianPInput to conditionally enable/disable expensive Morison hydrostatic calculations |
| modules/hydrodyn/src/Morison.f90 | Added optional calcHstLds parameter; refactored RefineElementHstLds_Cyl from recursive to iterative; updated function signatures to pass parameter struct p for accessing Gravity; contains spelling errors |
| modules/nwtc-library/src/NWTC_Base.f90 | Optimized SetErrStat to avoid redundant string operations when ErrStat is initially None |
| modules/nwtc-library/src/NWTC_Num.f90 | Removed unused error handling parameters (ErrStat, ErrMsg) from DCM_logMapD and DCM_logMapR subroutines |
| modules/nwtc-library/src/ModMesh_Mapping.f90 | Updated DCM_logmap calls to match new signature without error parameters |
| modules/nwtc-library/src/ModMesh.f90 | Updated DCM_logmap calls and removed associated error checking logic |
| modules/nwtc-library/src/NetLib/lapack/NWTC_LAPACK.f90 | Removed redundant ErrMsg = "" initialization |
| modules/wakedynamics/src/WakeDynamics.f90 | Updated DCM_logMap calls to match new signature |
| modules/seastate/src/SeaSt_WaveField.f90 | Removed redundant ErrMsg = "" initializations; renamed internal function parameter to avoid shadowing |
| modules/inflowwind/src/IfW_FlowField.f90 | Removed redundant ErrMsg = "" initializations |
| modules/moordyn/src/MoorDyn_Misc.f90 | Refactored getWaterKin using select case structure; improved code formatting; contains spelling errors |
| modules/aerodyn/src/FVW_Wings.f90 | Refactored if-else chain to select case; moved variable P4 initialization before P2 for better cache locality |
| modules/aerodyn/src/FVW_BiotSavart.f90 | Reduced nesting depth with early returns; moved Uind accumulation outside inner loop; added CPs_icp temporary to reduce array indexing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR is ready do merge.
Feature or improvement description
This PR contains changes which reduce computation time in HydroDyn, Morison, NWTC_Num, NWTC_Base. The largest improvement was adding a flag to HydroDyn_CalcOutput which can disable the hydrostatic load calculation in Morison_CalcOutput. This calculation is relatively expensive and unnecessary when calculating dYdx for HydroDyn when not perturbing displacement or orientation inputs. This change reduced computation time by 20% for the the 5MW_MRSemi_DLL_WSt_WavesIrr regression test.
The changes to NWTC_Base allow functions to skip setting ErrMsg = '' at the beginning because SetErrStat will ignore the contents if ErrStat is initially zero. As the size of ErrMsg has increased to handle longer message, it's now possible to see the time required to zero that memory when profiling and can account for 1-2% of the total runtime.
Similarly the changes to NWTC_Num removes the unused error handling from DCM_LogMap which also removes the ErrMsg = '' statement.
Some of the OLAF functions were modified slightly, moving variables out of loops, which resulted in a surprising 5% decrease in runtime.
Lastly, the calculation of the Morison hydrostatic loads was refactored to eliminate the need to recursively refine the points which gave another couple percent improvement. This is mostly due to not having to repeatedly allocate variables on the stack, which is typical of recursion.
Flame Graph before optimization for 10s run of 5MW_MRSemi_DLL_WSt_WavesIrr (9m 25s):

Flame Graph after optimization for 10s run of 5MW_MRSemi_DLL_WSt_WavesIrr (7m):

Test results, if applicable
Test results are unchanged.