Skip to content
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

Better encapsulation of compiler flag customization #3287

Closed
jgfouca opened this issue Nov 1, 2019 · 8 comments · Fixed by #4088
Closed

Better encapsulation of compiler flag customization #3287

jgfouca opened this issue Nov 1, 2019 · 8 comments · Fixed by #4088

Comments

@jgfouca
Copy link
Contributor

jgfouca commented Nov 1, 2019

Hi all,

I'm back to spending a lot of cycles look at our build system. One that I noticed is that there's some confusion for concept of customizing compiler flags.

If you want to customize compiler flags, there's many places to look depending on the context:

  • If you want a global customization for a file, it's done directly in tools/Makefile (or CMakeLists.txt) (e.g. rrtmg_lw_k_g.f90)
  • If you want compiler/platform specific changes, you use config_compilers.xml
  • If you want compiler/platform specific changes for a particular file, you use a Depends file

I'm thinking that it sure would be nice if all compiler flag customizations could be encapsulated by config_compilers.xml. We could enhance it to have filename selectors.

Thoughts?

@jedwards4b
Copy link
Contributor

I don't see any problem with this.

@mvertens
Copy link
Contributor

mvertens commented Nov 1, 2019

@jgfouca - I think this is a really good idea.

@rljacob
Copy link
Member

rljacob commented Nov 1, 2019

I like the idea of file-specific things (either global or compiler/platform specific) being in one config file but I'm concerned about config_compilers getting even larger.

@rljacob
Copy link
Member

rljacob commented Nov 1, 2019

Also filenames from way down deep in component model source code probably shouldn't be in the global config_compilers. This sounds like a config file that should live in cime_config.

@billsacks
Copy link
Member

I feel that component models should have a mechanism for setting file-specific flags for files in their builds that doesn't require touching cime. I think that's what @rljacob is suggesting, too, in his last comment:

Also filenames from way down deep in component model source code probably shouldn't be in the global config_compilers. This sounds like a config file that should live in cime_config.

I think he means the cime_config in each component, and I would agree with that. In addition to preventing components from needing to update cime whenever they want to change something file-specific, this would also prevent problems like: If one component has a file foo.F90 that needs certain compiler flags, what happens to some other component's foo.F90 compilation?

@jedwards4b
Copy link
Contributor

It should be straight forward to check for config_compilers.xml in each components cime_config directory and append it to the xml object if it's there. We already look for one in ~/.cime/config

@jgfouca
Copy link
Contributor Author

jgfouca commented Nov 1, 2019

@jedwards4b , @billsacks , that's a good idea.

@billsacks
Copy link
Member

@jedwards4b 's idea sounds good as long as it maintains a connection between file-specific settings and the component defining those settings. We could get some really hard to track down issues if the settings in one component impact the compilation of a different component (if there are duplicately-named files), so I want to be sure that we take pains to avoid that.

jgfouca added a commit that referenced this issue Sep 10, 2021
Change E3SM to use cmake macro file system

This is a big change for E3SM but should have no impact on other models and did not require big changes to CIME code.

This PR lays the groundwork for E3SM leaving the config_compilers.xml/BuildTools.configure behind for good, replacing that system with a Cmake-based cache file system that can, if needed, be used to generate a Makefile macro when needed (for some sharedlibs).

I will lay out detail documentation for this change when I create the corresponding PR in E3SM that activates these new macros.

Test suite: scripts_regression_tests (both with and without new macros in the host repo)
Test baseline:
Test namelist changes:
Test status: [bit for bit, roundoff, climate changing]

Fixes #3287
Fixes #3446
Fixes #3341
Fixes #2965

User interface changes?:

Update gh-pages html (Y/N)?:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants