New Build System #791
New Build System #791
Conversation
|
I decided that I'll fight with tl;dr: We have two options:
I'm strongly in favour of 2. I think these runs have flagged some genuine issues that are worth fixing now rather than sweeping back under a rug of compiler options. Or maybe some of these are fixed on Regarding those issues, they actually fall into some large categories.
The remaining tests ( |
|
Also probably worth mentioning that this is a continuation of this original external PR (#724). |
fecf573 to
971fa3c
Compare
|
I have rebased on main cleaned up some things (e.g. extracted #808). There are some test cases which run into issues. From what I could see running tests locally (i.e. without checksums), the ccsn, all rsp tests, and all adipls tests fail with FPEs. While these can easily be fixed by initializing certain variables to zero at somewhat arbitrary points, it doesn't feel like a clean solution. Moreover, adipls is not failing on some uninitialized variable, but on an actual divide by zero. I actually prefer going for option one, I have added a commit that disables FPEs by default. Syncing this PR with main was rather painful, so I would rather see this merged sooner than later (as long as there are no outstanding issues of course). Since the option to enabled FPEs still exists, getting the tests/other parts of MESA to cooperate can still be done in the future. I am running the test suite on the local cluster with the latest changes, and I'll see whether any other issues crop up. This is without the checksums, since they are not in the repository. |
|
Something I forgot to add is that the default insertion of NaNs could have also been a reason for the difference in the photos. |
ccbe028 to
d248833
Compare
This commit introduces a new build system (still based on make files). Reasons for replacing the previous build system were: - the previous build system had a lot of duplication between the modules, both in terms of make files and shell scripts. - parallelisation was limited. The new build system parallelises better within a module, and between modules. - build scripts and output from the build ended up in the same folder. With the new system, all build output files are stored in the folder ``build`` in the repository root, making cleaning build files equivalent to deleting this folder For the details of the implementation, see the newly added documentation (Developing -> Build system). Some further notable changes: - makedepf90 has been replaced by a perl script that scans the fortran source files. This allows for better flexibility in construction the make files. - Currently, `pymesa` does not work with the new build system, as the new system does not provide a way to build shared libraries for every module. - Dependencies are being discovered using pkg-config. This change makes it easier to build MESA without the SDK, as linux distributions typically ship with pkg-config files.
This commit adds the necessary pkg-config files until the SDK ships them.
Just to confirm that you're absolutely right: these fail on So I think this looks ready to merge? It's a big change so we probably want an extra set of eyes, especially from the build side. @rhdtownsend? |
|
Ah, I now realise I haven't reviewed the actual code fixes. I'll have a look as soon as I get a chance and put in a proper GitHub review. |
|
"there is already a bunch of documentation in this PR on how the new build system works with also information on how to use a module in another project." Can this information make its way into some excerpt on the mesadocs website? Ultimately, anything explained inside this pr will be lost to history in due time. Perhaps under "modules", there can be a small section titled "build system" that rehashes this. |
|
Super happy to see the new build system reach it's final form! Given the MESA release candidate is coming very soon. At the moment, I am hesitant toward merging this in until after the release comes out, unless we delay the release. Imo, the new build system should live in main for some months before making into a new release. Are my concerns valid, and or @VincentVanlaer is your preferred timeline such that you'd prefer this to make it immediately into the next release? Am i just fearmongering? let me know. Perhaps, this is something @VincentVanlaer can discuss with mesa-dev. |
|
@VincentVanlaer have you been able to build on MacOS? I get a |
By in this PR, I mean actual changes to the documentation. So not the comments on this or the previous PR or commit messages. I've put it under developing. |
|
Thanks, I found it! |
Somewhat of a concern I have, is that there is some upkeep for not merging this branch, as it touches all modules and conflicts do arise. But I do agree that it is preferable to not have something that is broken for a certain subset of OSes (e.g. MacOS, various Linux installations, or maybe even certain minimal environments). I can see if I can extract some changes from this branch to reduce the number of conflicts arising.
|
There was some early testing, but I don't own a mac, so I can't check myself. I am not sure exactly what is provided by the mesasdk on macos, and what is provided by the OS itself (and what the versions of those things are). My suspicion is that sed is not available? |
Mac does have I'll have a look at this too. It builds in my linux system |
|
In the meantime, I have set up a MacOS VM, and have gotten everything to work, all the way to running the default work dir with pgplot showing up. Some notes:
|
|
A few issues on my mac, and I was wondering what your thoughts are @VincentVanlaer in avoiding these:
Finally, if I remove the use of the above, my current install is failing at: |
This is not available in the MacOS make version, which seems to be perpertually locked at 3.81
Old version of make (e.g. the 3.81 version that MacOS uses) drops trailing slashes, which makes it impossible for us to distinguish normal files and directories.
This was introduced in make 4, and is therefore not available in 3.81
Make version is too old
|
Yeah I wasn't entirely clear in my previous comments. I came across the same issues, and temporarily fixed them by switching to a newer version of gnumake. However, I still needed to fix them properly. This has been done now, and the new build system works in a MacOS VM. The only thing you should need to do compared to main is install pkg-config. |
|
It installs on my macOS! I did have to do a |
|
@VincentVanlaer the install process creates a |
|
Hmm, I doesn't do that for me. Are these leftover from a previous install maybe? I think I removed the sample directory at some point, so changing to this branch probably removed all the files except for the untracked ones. |
|
Nice! Some timings on my macOS: |
|
I am getting a warning: |
|
Looks like I have made a typo in the pgplot pkg-config file. Should have been |
This is to ensure the correct runpath is set by the gfortran wrapper in the sdk
|
Should be fixed now. I have also added a fix for the correct selection of the math libraries in the SDK. |
Transitive dependencies are now only added at the linking stage, not the compile stage
Old MacOS versions don't have realpath
by Vincent Vanlaer