Skip to content

Conversation

@mpaiao
Copy link
Contributor

@mpaiao mpaiao commented Nov 26, 2025

Description

This PR is a follow-up of @mdietze comment in #410, which I agreed. We had too many tracked include.mk files, and most of them have gone stale. So instead of having a lot of files on GitHub, I kept a few as examples on how to configure these files when using Intel, GNU and PGI compilers, plus one for a MacOS system.

This PR also simplifies many of the preprocessor code in ED2, to be consistent with the current options. In this refactoring, I took two variables that were inconsistently defined in the include.mk files and formalised them a bit:

CMACH now defines the operating system, it currently takes the following options:

  • LINUX For Linux systems
  • MACOS For MacOS systems

FC_TYPE now defines the family of compilers used. It currently takes the following options:

  • INTEL For ifx/icx (or ifort/icc in older systems)
  • GNU For gfortran/gcc
  • PGI For pgfortran/pgcc.

And now the Makefile checks for the values and should graciously fail if the value is not one of the above. My main motivation for this was that CMACH was a mix of OS and compiler, and that was just propagating lots of preprocessor if statements in the code. For most cases, the test is either compiler-dependent or OS-dependent, so it makes sense to have two variables handling these.

Notes.

  1. For now I kept the PGI because I think @yjkim1028 used to use it, but I don't know if this is still the case. My understanding is that PGI is no longer a maintained compiler, so if no one needs it, we can drop PGI too.
  2. There was some bits of preprocessing code for Windows OS. I kept the code because they were a bit more involving, but we currently do not support Windows compilation and CMACH cannot be set to Windows. If anyone wants to try to enable it, I am happy to add it.
  3. I did remove several systems (e.g., CRAY, HP, IBM, SUNHPC and a few others) because they were not consistently used and they may have been code legacy that came with BRAMS. I may have removed more than what I should, but happy to add back systems if there is such need.

Collaborators

Initial idea from @mdietze

Types of changes

  • Hot fix (emergential bug fix to make ED2 run again)
  • 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)

Changes in Settings, Input Files or Output Files

  • This feature requires changes in ED2IN
  • This feature requires changes in the XML parameter file
  • This feature requires changes in other input files
  • This feature will change output files

Most users will need to revise their include.mk files. I updated .gitignore so it will not track any other include.mk besides the ones in this PR, but people should feel free to create their own.

Expectation of Answer Changes:

  • No changes expected (bit-for-bit compatibility)
  • Changes expected with specific configurations (see details below)
  • Changes expected in all simulations (see details below)

Checklist:

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

Testing :

  • All new and existing tests passed.

@mpaiao mpaiao marked this pull request as draft November 26, 2025 13:23
@mpaiao
Copy link
Contributor Author

mpaiao commented Nov 26, 2025

I see that one of the CI tests is failing due to a FPE error that is happening at the disturbance routine. I cannot reproduce this error on MacOS (using all the debugging flags), and none of the changes in this PR should affect the disturbance routine directly.

This is either some latent bug or (more likely), one of the refactoring steps is incorrect. I will try to run the model in a different system and see if I get to replicate the error. In the meantime, let's keep this PR as a draft.

…eed special flags to

any of them, we can add them. For now they are all assigned to Linux.
2. Fix some of the Makefile commands to issue errors when something was incorrectly set.
@mpaiao
Copy link
Contributor Author

mpaiao commented Nov 28, 2025

I dropped the special options for Docker and continuous integration from CMACH, because there was no clear need for them. I didn't expect that to make any difference, but the test that previously failed now worked fine...

I also fixed some of the new configuration checks in Makefile and it is now working as intended. I will drop the draft tag, I think this is ready for review.

@mpaiao mpaiao marked this pull request as ready for review November 28, 2025 18:10
@mpaiao
Copy link
Contributor Author

mpaiao commented Dec 1, 2025

Thanks for revising it @mdietze I will go ahead and merge it.

@mpaiao mpaiao merged commit 7dee054 into EDmodel:master Dec 1, 2025
4 of 6 checks passed
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