Skip to content

Conversation

mpaiao
Copy link
Contributor

@mpaiao mpaiao commented Dec 31, 2024

Update linear system solvers with lapack libraries for improved efficiency.

Description

ED2 used to have a fairly robust linear system solver, however, it used Gaussian elimination for every case, which is not efficient. This pull requests replaces the built-in solver with the Linear Algebra Package (LAPACK) subroutines. LAPACK is a well-known and widely available library, so this change should have minimal impact on the ability to build ED2 in most local and high-performance computing systems.

This change will require new variables LAPACK_INCS and LAPACK_LIBS to be set in include.mk.[machine] files. If Lapack is installed in regular paths (e.g., /usr/local), the only addition needed is the list of libraries:

LAPACK_INCS=
LAPACK_LIBS=-llapack -lblas

Otherwise, the full path may be needed. For example, in MacOS systems using HomeBrew:

LAPACK_PATH=/usr/local/opt/lapack
LAPACK_INCS=-I$(LAPACK_PATH)/include
LAPACK_LIBS=-L$(LAPACK_PATH)/lib -llapack -lblas

I added templates in all tracked include.mk, but they may need to be adjusted.

Collaborators

@femeunier and @rgknox suggested this a while back. @robkooper because I may need to coordinate these edits to keep Docker functional.

Motivation and Context

This change removes the need for computational burdensome matrix inversion algorithms for simpler matrices, and allows leveraging efficient and well tested libraries for solving linear systems in ED2. This is called rather frequently (radiation time step), so using lapack should reduce the runtime.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

I will update the documentation if this pull request is merged.

Testing :

  • All new and existing tests passed.

…This should increase

the computational efficiency and allow ED2 to leverage future developments from lapack.
@mpaiao mpaiao requested a review from robkooper January 2, 2025 21:58
@mpaiao
Copy link
Contributor Author

mpaiao commented Jan 2, 2025

@femeunier would you mind revising this one too as you had originally talked about using lapack (a while ago...)?
I tried to list you but somehow it didn't let me add you there. Thanks for considering!

@mpaiao mpaiao requested review from mdietze and xiangtaoxu May 1, 2025 01:47
Copy link
Contributor

@mdietze mdietze left a comment

Choose a reason for hiding this comment

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

kinda nuts that most of the changes were to all the various versions of include.mk. Feels like we could probably clear our a lot of those, have institutions keep local versions, and only retain a small number of canonical versions?

# If you don't have it, leave USENC=0 and type a dummy
# folder for NC_LIBS (e.g. -L/dev/null or leave it blank)
USENC=0
NC_LIBS=-L/dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why adding LAPACK involve the removal of hdf5 and netcdf info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just some general clean up. I deleted these because ED2 doesn't need NetCDF libraries, this is either legacy from borrowing the include.mk files from BRAMS, or some short-lived attempt to use NetCDF with ED2 a very long time ago. Same thing for HDF, note that the deleted one is for HDF4, not HDF5.

@mpaiao
Copy link
Contributor Author

mpaiao commented May 3, 2025

kinda nuts that most of the changes were to all the various versions of include.mk. Feels like we could probably clear our a lot of those, have institutions keep local versions, and only retain a small number of canonical versions?

Yes for the clearing out, I think we could delete those specific to cluster, and keep generic examples for different compilers (and maybe OS if they are any different from the compiler ones).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants