diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 04815c18..9476d6e8 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -10,10 +10,11 @@ To indicate your agreement, add your details to the the following table. If you are not submitting contributions on behalf of an organisation please use "n/a" for your affiliation. -| GitHub Username | Real Name | Affiliation | -|-----------------|-----------------|-------------| -| MatthewHambley | Matthew Hambley | Met Office | -| yaswant | Yaswant Pradhan | Met Office | +| GitHub Username | Real Name | Affiliation | +|-----------------|-----------------|----------------------------------| +| MatthewHambley | Matthew Hambley | Met Office | +| yaswant | Yaswant Pradhan | Met Office | +| hiker | Joerg Henrichs | Bureau of Meteorology, Australia | --- diff --git a/Documentation/source/fab_base/config.rst b/Documentation/source/fab_base/config.rst index 0152a4b8..938072d1 100644 --- a/Documentation/source/fab_base/config.rst +++ b/Documentation/source/fab_base/config.rst @@ -250,3 +250,98 @@ instance that uses other shells. Usage: nc_flibs = [] linker.add_lib_flags("netcdf", nc_flibs) + +Application-specific settings +============================= +Besides site-specific settings, the Fab base class also allows to use +application-specific setups, which can work together with site-specific +configurations using inheritance. These config files are the same +as site-specific configuration files described previously, but are +imported from the directory ``app_specific``. + +An example of this is LFRic. The infrastructure (lfric_core) repository +contains site-specific configuration. For example, they will define +the required compilation flags for files. These settings will be used +even for applications in applications in the lfric_apps repository. +But certain applications needs additional flags. For example, the +lfric_atm application will compile the UM physics code, and this require +that by default any real values are double precision (and in some cases +file-specific work arounds for compiler bugs. To avoid that the site-settings +from lfric_core need to be duplicated, the following structure is +recommended (and used in lfric_atm), in this example for the site +`nci` on the platform `gadi` - the arrows indicating an 'inherit from' +relationship:: + + SiteConfig/default <- AppConfig/default + ^ ^ + | | + SiteConfig/NciGadi <- AppConfig/NciGadi + +At start up, the application-specific configuration for the specified site +will be read in. The Python method resolution order then guarantees that +any ``super()`` access will first call ``AppConfig/default``, which will +then call ``SiteConfig/NciGadi``, and then ``SiteConfig/default``. + +In Python code, this looks as follows: + +``app_specific/nci_gadi``: + +.. code-block:: python + + from app_specific.default.config import Config as ConfigAppDefault + from site_specific.nci_gadi.config import Config as ConfigSiteNciGadi + + class Config(ConfigAppDefault, ConfigSiteNciGadi): + def __init__(self): + super().__init__() + +``app_specific/default:`` + +.. code-block:: python + + from site_specific.default.config import Config as ConfigSiteDefault + + class Config(ConfigSiteDefault): + def __init__(self): + super().__init__() + +``site_specific/nci_gadi:`` + +.. code-block:: python + + from site_specific.default.config import Config as ConfigSiteDefault + + class Config(ConfigSiteDefault): + def __init__(self): + super().__init__() + +``site_specific/default:`` + +.. code-block:: python + + class Config: + def __init__(self): + ... + +This setup will allow to reuse site-specific setup, which can be overwritten +by application-specific settings. As an example of what to do on what level: + +1. ``site_specific/default`` would define optimisation levels (depending on profile) +2. ``site_specific/nci_gadi`` could add flags for more thorough full-debug tests. + It would also contain all required library definitions. +3. ``app_specific/default`` would add flags for compiling UM (e.g. 8 byte default reals) +4. ``app_specific/nci_gadi`` could add additional compiler optimisation flags for + certain files, which are beneficial for the resolution usually used at NCI. Also, + if an application needs additional libraries, they can be added here. + +The usage of ``nci_gadi`` means that additional compiler flags can easily be +added, since it will only affect runs on NCI. If a flag would be useful for +any site (e.g. to work around a compiler bug), this flag would eventually be moved +into the ``default`` setup. + +.. important:: + If there is a application-specific configuration, it is important that + each site specifies its own application-specific setup. Otherwise only + the site-specific configuration would be used (since the import from + ``app_specific/SITE`` fails, which means that the application specific + setup would not be executed at all). diff --git a/source/fab/fab_base/fab_base.py b/source/fab/fab_base/fab_base.py index d8799350..09740f41 100755 --- a/source/fab/fab_base/fab_base.py +++ b/source/fab/fab_base/fab_base.py @@ -201,7 +201,7 @@ def root_symbol(self) -> list[str]: def name(self) -> str: ''' - :returns: the name of the apps. + :returns: the name of the app. ''' return self._name @@ -329,12 +329,6 @@ def setup_site_specific_location(self) -> None: self.logger.warning("Could not find caller directory, " "defaulting to '.'.") - # We need to add the 'site_specific' directory to the path, so - # each config can import from 'default' (instead of having to - # use 'site_specific.default', which would hard-code the name - # `site_specific` in more scripts). - sys.path.insert(0, str(dir_caller / "site_specific")) - def define_site_platform_target(self) -> None: ''' This method defines the attributes site, platform (and @@ -379,15 +373,19 @@ def site_specific_setup(self) -> None: ''' self.setup_site_specific_location() try: - config_name = f"site_specific.{self.target}.config" + config_name = f"app_specific.{self.target}.config" config_module = import_module(config_name) - except ModuleNotFoundError as err: - # We log a warning, but proceed, since there is no need to - # have a site-specific file. - self._logger.warning(f"Cannot find site-specific module " - f"'{config_name}': {err}.") - self._site_config = None - return + except ModuleNotFoundError: + try: + config_name = f"site_specific.{self.target}.config" + config_module = import_module(config_name) + except ModuleNotFoundError as err: + # We log a warning, but proceed, since there is no need to + # have a site-specific file. + self._logger.warning(f"Cannot find site-specific module " + f"'{config_name}': {err}.") + self._site_config = None + return self.logger.info(f"fab_base: Imported '{config_module.__file__}'.") # The constructor handles everything. self._site_config = config_module.Config() diff --git a/tests/unit_tests/fab_base/app_specific/default/config.py b/tests/unit_tests/fab_base/app_specific/default/config.py new file mode 100644 index 00000000..7d803f98 --- /dev/null +++ b/tests/unit_tests/fab_base/app_specific/default/config.py @@ -0,0 +1,24 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +""" +Example of an app-specific default config class. +""" + +# Mypy does not handle the relative import here properly, ignore error: +from site_specific.default.config import Config as ConfigSiteDefault # type: ignore + + +class Config(ConfigSiteDefault): + """A simple app-specific default configuration. It inherits + from the site-specific default configuration. + """ + + def __str__(self): + """ + This str method also collects the call-order. + """ + return f"AppSpecificDefault -> {super().__str__()}" diff --git a/tests/unit_tests/fab_base/app_specific/site_platform/config.py b/tests/unit_tests/fab_base/app_specific/site_platform/config.py new file mode 100644 index 00000000..4e1bb707 --- /dev/null +++ b/tests/unit_tests/fab_base/app_specific/site_platform/config.py @@ -0,0 +1,28 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +""" +Example of an app- and site-specific default config class. +""" + +# Mypy does not handle the relative import here properly, ignore error: +from app_specific.default.config import Config as ConfigAppDefault # type: ignore +from site_specific.site_platform.config import Config as ConfigSiteSitePlatform # type: ignore + + +class Config(ConfigAppDefault, ConfigSiteSitePlatform): + """A simple app-specific configuration for a specific site/platform. + It inherits from both the default app-specific config and the + site-specific configuration. The order of the base classes is important + to achieve the expected call sequence across all classes: + this -> AppSpecificDefault -> SiteSpecificConfig -> SiteSpecificDefault + """ + + def __str__(self): + """ + This str method also collects the call-order. + """ + return f"AppSpecificSitePlatform -> {super().__str__()}" diff --git a/tests/unit_tests/fab_base/site_specific/default/config.py b/tests/unit_tests/fab_base/site_specific/default/config.py index bee186f2..18686da4 100644 --- a/tests/unit_tests/fab_base/site_specific/default/config.py +++ b/tests/unit_tests/fab_base/site_specific/default/config.py @@ -22,6 +22,9 @@ class Config: def __init__(self): self._args = None + def __str__(self) -> str: + return "SiteSpecificDefault" + @property def args(self) -> argparse.Namespace: ''' diff --git a/tests/unit_tests/fab_base/site_specific/site_platform/config.py b/tests/unit_tests/fab_base/site_specific/site_platform/config.py new file mode 100644 index 00000000..57f4b9f1 --- /dev/null +++ b/tests/unit_tests/fab_base/site_specific/site_platform/config.py @@ -0,0 +1,24 @@ +############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +############################################################################## + +""" +Example of an site_specific non-default config class. +""" + +# Mypy does not handle the relative import here properly, ignore error: +from site_specific.default.config import Config as ConfigSiteDefault # type: ignore + + +class Config(ConfigSiteDefault): + """A simple site-specific configuration for a given site/platform. + It inherits from the site-specific default configuration. + """ + + def __str__(self) -> str: + """ + This str method also collects the call-order. + """ + return f"SiteSpecificSitePlatform -> {super().__str__()}" diff --git a/tests/unit_tests/fab_base/test_fab_base.py b/tests/unit_tests/fab_base/test_fab_base.py index 160d8c93..d25e8750 100644 --- a/tests/unit_tests/fab_base/test_fab_base.py +++ b/tests/unit_tests/fab_base/test_fab_base.py @@ -355,9 +355,7 @@ def test_site_specific_outside_dir(monkeypatch) -> None: old_path = sys.path[:] monkeypatch.setattr(sys, "argv", ["fab_base.py"]) _ = FabBase(name="test-help") - assert sys.path[2:] == old_path - assert str(this_dir / "site_specific") in sys.path[0] - assert str(this_dir) in sys.path[1] + assert sys.path == [str(this_dir)] + old_path def test_site_specific_inside_dir(monkeypatch) -> None: @@ -371,8 +369,40 @@ def test_site_specific_inside_dir(monkeypatch) -> None: monkeypatch.setattr(sys, "argv", ["fab_base.py"]) monkeypatch.setattr(inspect, "stack", lambda: []) _ = FabBase(name="test-help") - assert sys.path[1:] == old_path - assert "site_specific" == sys.path[0] + assert sys.path == old_path + + +def test_app_specifc(monkeypatch) -> None: + ''' + Tests that an app_specific directory works as expected. + The setup in the test dir is: + site_specific/default/config + site_specific/site/config + app_specific/default/config + app_specific/site/config + The last class uses multiple inheritance: + config(AppSpecificDefaultConfig, SiteSpecificSiteConfig) + + With each method calling super(), the following call order + should happen: + AppSpecificSite + --> AppSpecificDefault + --> SiteSpecificSite + --> SiteSpecificDefault + This allows an app-specific setup to modify the settings from + site-specific setup etc. + This test calls ``__str__``, which goes through all base classes + to assemble a string that represents the order in which the base + classes are called. + ''' + monkeypatch.setattr(sys, "argv", ["fab_base.py", "--site", "site", + "--platform", "platform"]) + monkeypatch.setattr(inspect, "stack", lambda: []) + fab_base = FabBase(name="test-help") + + assert (str(fab_base.site_config) == + "AppSpecificSitePlatform -> AppSpecificDefault -> " + "SiteSpecificSitePlatform -> SiteSpecificDefault") def test_build_binary(monkeypatch) -> None: