search

MESAHub / mesa

Modules for Experiments in Stellar Astrophysics
http://mesastar.org
GNU Lesser General Public License v2.1
145 stars 39 forks source link

Cleaning up build system #662

Open VincentVanlaer opened 5 months ago

VincentVanlaer commented 5 months ago

Context

I am experimenting with building and packaging MESA for nixos using nix. I did the same with GYRE recently. In order to do that properly, I wanted to understand how MESA actually gets build. I found the existing combination of bash scripts and makefiles difficult to understand and adapt, so I ended up re-implementing the build system mostly from scratch. This issue is for me to check if what I'm proposing below is considered a meaningful improvement.

What I found to be not ideal in the current build system

What I ended up implementing

The implementation, with a few modules (const, utils, math, mtx, chem) ported can be found on https://github.com/VincentVanlaer/mesa/tree/new-build. The main flow is as follows:

Makefile (in repository root)  -> single Makefile in each module -> makefiles in the make subfolder doing the heavy lifting

The system has the following benefits:

In terms of lines of code, the changes are 162 files changed, 563 insertions(+), 2888 deletions(-) for the five modules ported.

Caveats & remarks

Conclusions

I have two questions after all of this (which are difficult to answer for me given that I have written them):

It might also be worth considering alternatives such as only applying some of the concepts, or porting to another build system such as Meson.

If you want to play with the Makefiles I can also upload a patched version of the mesasdk that contains the necessary changes and a gnumake install with at least version 4.3.

fxt44 commented 5 months ago

wow! impressive that rather than just complain about the current build system, an experiment with an alternative build system was pursued and implemented. i have no comments on your two questions at this point ; still absorbing the newness and scope of this development.

mjoyceGR commented 5 months ago

This is brilliant Vincent! Was especially amused/impressed to see this post after midnight in the middle of the summer school :)

warrickball commented 5 months ago

First, thanks for taking this on. I think your observations are sound and having a demonstration of what things could look like will hopefully kickstart the process of fixing things.

I think it's worth tagging @rhdtownsend as the resident build system expert.

I'm going to boldly chime in, having only looked around for about 20 min, because I maintain the ifort-based tests we run, so I've fought with utils/makefile_header a bit over the years and at a glance I think the breakup of different scripts in $MESA_DIR/make will make non-standard builds (e.g. using HPC modules) easier.

I have two questions after all of this (which are difficult to answer for me given that I have written them):

  • Are the new makefiles and scripts actually cleaner?

Yes, very yes. The module makefile_bases duplicate a lot of code so simply refactoring that, as you have done, reduces those files. I never saw much value in cluttering the module directories with wafer-thin wrappers around make (e.g. clean, mk) and its appealing that, for the chem module,

the old structure ```` $ tree chem/ chem/ ├── build_and_test ├── build_and_test_parallel ├── build_data_and_export ├── clean ├── data │   ├── isotopes.data │   ├── lodders03.data │   ├── lodders09.data │   └── README ├── export ├── i1 ├── i1p ├── make │   ├── makefile │   └── makefile_base ├── mk ├── preprocessor │   ├── build_for_export │   ├── chem_input_data.tar.xz │   ├── clean │   ├── inlist │   ├── make │   │   └── makefile │   ├── mk │   ├── README │   ├── rebuild_all │   ├── rn │   └── src │   ├── chem_support.f90 │   └── create_table.f90 ├── private │   ├── chem_isos_io.f90 │   ├── lodders_mod.f90 │   └── nuclide_set_mod.f90 ├── public │   ├── chem_def.f90 │   └── chem_lib.f90 └── test ├── chem_ids.list ├── ck ├── clean ├── cleanup ├── export ├── make │   ├── makefile │   └── makefile_base ├── mk ├── mkx ├── rn ├── src │   └── test_chem.f └── test_output ````

is at first glance reduced to

the new one. ```` $ tree chem/ chem/ ├── Makefile ├── preprocessor │   ├── chem_input_data.tar.xz │   ├── inlist │   ├── Makefile │   ├── README │   └── src │   ├── chem_support.f90 │   └── create_table.f90 ├── private │   ├── chem_isos_io.f90 │   ├── lodders_mod.f90 │   └── nuclide_set_mod.f90 ├── public │   ├── chem_def.f90 │   └── chem_lib.f90 └── test ├── chem_ids.list ├── src │   └── test_chem.f └── test_output ````

I'll note that I see less obvious gain over the structure of the old utils/makefile_header. The old one is ~340 lines, compared with the total contents of make now being ~272 lines. Still, just breaking it up makes it easier to navigate.

  • If so, is it worth the further effort of development and testing to swap things over?

I personally think yes, with the caveat that I might be underestimating how much effort remains.

It might also be worth considering alternatives such as only applying some of the concepts, ...

When I first start looking at this, I also wondered about cherry-picking some ideas, but I think everything you've implemented has clear benefits. If in each module, we consolidate makefile and makefile_base, remove the wafer-thin wrapper scripts and refactor what's left, I think we end up with more or less what you've got.

... or porting to another build system such as Meson.

I'm personally a big fan of trying to accomplish as much as possible with the simplest tools that are available. More advanced build tools have a place but I don't think our requirements are complicated enough to warrant the nuisance that might be caused for someone trying to install somewhere like an HPC system, with an old OS and little room to install base packages.

Regarding potential improvements, this stands out:

  • Modules that do not depend on each other are built in parallel

I like the sound of that! MESA's current build time is ~15-20 minutes. Just cutting that in half would feel like a big improvement on the CI turnaround.

I have three points:

  1. What feature of GNU make means you need at least version 4.3? I just had a look on our HPC and found 4.2.1, though 4.3 is available as a module. If it isn't too hard to workaround, I'd prefer relaxing that requirement by one or two minor versions, though I'm happy to keep the minimum requirement if it prevents things getting messy.
  2. We should preserve as much of the user-friendliness of the top-level install scripts as possible, including e.g.
    • warning that the SDK hasn't been initialised,
    • a single command to produce output to attach to troubleshooting queries and
    • detecting and stopping users from trying to run as sudo. I think e.g. ./help can be replaced with make help (or any other subcommand name) but the functionality must be somewhere.
  3. How much more work do you need to do to finish converting the build system, and how might the development team or community potentially make it easier for you? I'm personally happy to spend some very scarce time on seeing how to build with ifort/other non-standard builds under a new system. (I occasionally also built with the Fedora gfortran and libraries if gfortran got ahead of the SDK.)
rhdtownsend commented 5 months ago

Jumping in here with a few remarks. I've recently done quite a bit of work in revamping the build system for GYRE (and other projects I maintain). The changes include

The most recent release of GYRE (7.2) has many of these improvements; I'm going to be making another release (7.2.1) very soon, which will complete the update. It's this newer release that will make it into MESA as the GYRE refresh. Within the GYRE distribution directory, the top-level Makefile defines user-settable flags and various targets (all, install, clean, test, etc). The building all happens in the build/ subdirectory. The system is pretty clean, and robust against crazy stuff happening. And, it parallelizes very well, thanks to separate passes for module (.mod/.smod) file generation and code compilation.

ETA: oh, and not a single shell script apart from the SDK version checker!

rhdtownsend commented 5 months ago

On the subject of GNU make 4.3, it would be possible to bundle make into the SDK (I've done that in the past with 'custom' SDKs). One of the big changes in 4.3 was the introduction of grouped targets, which can cut down on makefile verbosity (see https://www.gnu.org/software/make/manual/html_node/Multiple-Targets.html). But the new GYRE build system doesn't need this, as the automatic dependency generation obviates the need for grouped targets.

rhdtownsend commented 5 months ago

A further note on the GYRE build system: all of the heavy lifting is done in the file build/Make.inc. This file handles all of the dependency generation, compilation rules, etc. build/Makefile just sets up the list of source files, libraries, executables. It would be possible to adapt this to MESA, with a single Make.inc for everything, and then a build/Makefile for each MESA module.

VincentVanlaer commented 5 months ago

Thanks for the positive feedback! I left some replies to the comments below

  1. What feature of GNU make means you need at least version 4.3? I just had a look on our HPC and found 4.2.1, though 4.3 is available as a module. If it isn't too hard to workaround, I'd prefer relaxing that requirement by one or two minor versions, though I'm happy to keep the minimum requirement if it prevents things getting messy.

It is indeed the grouped targets, which I introduced at some point to make parallel building work. Also because I currently don't understand how make deals with non-grouped multiple outputs.

  1. How much more work do you need to do to finish converting the build system, and how might the development team or community potentially make it easier for you? I'm personally happy to spend some very scarce time on seeing how to build with ifort/other non-standard builds under a new system. (I occasionally also built with the Fedora gfortran and libraries if gfortran got ahead of the SDK.)

It depends on how custom the other modules are. If they fit within the parameters of the new system as it is now, then it is not too much work. Maybe it would be a good idea for one or more developers to try to port some of the modules to the new system to become more familiar with it. It would also be great to see some tests with other compilers. In principle this should only require swapping out make/compile-settings-gnu.mk.

  • migrating from fpx3 to fypp as the pre-processor/templating engine. fpx3 required Perl, and hasn't been maintained for years; fypp is a maintained project (but does require Python 3).

  • replacing makedepf90 with a combination of fypp_deps (a wrapper around fypp that handles .fypp file dependency generation) and makedepf08.awk (an awk script that handles .f90 file dependency generation).

Since MESA doesn't use a templating engine like fpx3 (as far as I could tell at least), do you think it still make senses to replace makedepf90?

I'll definitely read through that.

And, it parallelizes very well, thanks to separate passes for module (.mod/.smod) file generation and code compilation.

That would indeed simplify things and could indeed remove the need for grouped targets

VincentVanlaer commented 5 months ago

I made the following two changes in the meantime

warrickball commented 4 months ago

@VincentVanlaer, I just made a first attempt to build with the new system. I cloned your fork, switched to the new-build branch, loaded MESA SDK 23.7.3 and ran make in the top-level MESA directory (which I've exported as MESA_DIR). This immediately failed with:

$ make
make[1]: Entering directory '/home/wball/mesa/vv-new-build/const'
makedepf90 -free -m modules/%m.mod -B ../build/const  -I public:private public/const_def.f90 public/const_lib.f90 test/src/test_const.f | \
INSTALL_INCLUDES='' \
MODULES='../build/const/modules/const_def.mod ../build/const/modules/const_lib.mod' \
BUILD_DIR='../build/const' \
../make//gen-compile-tree > ../build/const/depend

makedepf90: ERROR: Unknown Option '-B'
compile.mk:33: ../build/const/depend: No such file or directory
make[1]: *** [compile.mk:27: ../build/const/depend] Error 1
make[1]: *** Deleting file '../build/const/depend'
make[1]: Leaving directory '/home/wball/mesa/vv-new-build/const'
make: *** [build/depend:2: const] Error 2

Indeed, -B isn't an option for makedepf90, though -b is. Is that the intention? If I change -B to -b in make/compile.mk, I instead get

$ make
mkdir -p build
make/gen-folder-deps "make" const utils math mtx chem > build/depend
make[1]: Entering directory '/home/wball/mesa/vv-new-build/const'
mkdir -p ../build/const
makedepf90 -free -m modules/%m.mod -b ../build/const  -I public:private public/const_def.f90 public/const_lib.f90 test/src/test_const.f | \
INSTALL_INCLUDES='' \
MODULES='../build/const/modules/const_def.mod ../build/const/modules/const_lib.mod' \
BUILD_DIR='../build/const' \
../make//gen-compile-tree > ../build/const/depend
mkdir -p ../build/const/lib/pkgconfig/
make[1]: *** No rule to make target '../build/const/public/const_def.o', needed by '../build/const/lib/libconst.a'.  Stop.
make[1]: Leaving directory '/home/wball/mesa/vv-new-build/const'
make: *** [build/depend:2: const] Error 2

but that might not be surprising.

VincentVanlaer commented 4 months ago

I guess that the version of makedepf90 is older than the one I have. I also had to patch makedepf90 to make include files cleaner to support, but that shoudn't cause the error. In any case, I have been cleaning up and documenting the new build system, and I have replaced makedepf90 by a simple script in the meantime, similar to what Rich has done for GYRE. I'll make at least a draft PR in the next few days (depending on how fast I go over the remaining modules I had ported already), which should work with the SDK as it currently is.

rhdtownsend commented 4 months ago

FYI, the version of makedepf90 that ships with the SDK has also been patched, to add an ‘-X’ flag that adds dependencies on external modules. Patch file is attached.

cheers,

R

On Jul 30, 2024, at 8:55 PM, Vincent Vanlaer @.***> wrote:

I guess that the version of makedepf90 is older than the one I have. I also had to patch makedepf90 to make include files cleaner to support, but that shoudn't cause the error. In any case, I have been cleaning up and documenting the new build system, and I have replaced makedepf90 by a simple script in the meantime, similar to what Rich has done for GYRE. I'll make at least a draft PR in the next few days (depending on how fast I go over the remaining modules I had ported already), which should work with the SDK as it currently is. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

-- Rich Townsend • Professor of Astronomy Astronomy Department • University of Wisconsin-Madison Phone: 608-262-1752 • E-mail: @.***

fxt44 commented 4 months ago

i don't see a patch file.

rhdtownsend commented 4 months ago

Must have been stripped from the mail. Here it is:

makedepf90-sdk2.patch