How to find lfric core

[hiker]

@MatthewHambley , @yaswant , @Pierre-siddall , @t00sa , @cameronbateman-mo, @jasonjunweilyu

As part of Matthew's review, he raised an issue that boils down to: how do we know where lfric_core is when building an application.

FWIW, the current (makefile) solution is to use an environment variable (some stuffing around in local_build.py, but as far as I can see it eventually just exports CORE_ROOT_DIR and the calls make).

I had the following design goals in mind when creating our solution:

1. It should be obvious when studying the script how the source is found (and setting an environment variable in a script which then calls make is hiding this pretty well ;) ).
2. the way an application is build should be independent of the location, e.g. the same command in core/applications/... and apps/applications/... should work
3. Don't require any special setup (e.g. atm the makefiles set PYTHONPATH to ensure psyclone_tools.py can be imported. This needs to be done somewhere :)

There are various solutions of course. Here what Jason and I decided to do (note that this was originally motivated by not having a Fab release, but our approach allows additional things):

Any fab script is called using a wrapper in core, e.g.:

$LFRIC_CORE/build.sh  ./fab_lfric_atm.py ....

The wrapper is then used to adjust PYTHONPATH so that lfric_base from the core repo and all other tools can be easily imported. This works the same for applications in core and apps.

Matthew told us in the core review:

I'm not sure what the build.sh script is for. It looks like left overs from before Fab was installed on the Python path. Either way, one of the things we want to achieve with a change in build system is to get rid of pointless wrapper scripts so this needs to go.

The wrapper is definitely not pointless, since it connects the application directory with the core directory. Supporting the import of Fab can of course be removed, as long as we are using a released version. For testing new versions of Fab this is still pretty handy (esp. ih this repo, where we evaluate new Fab versions all the time :) ), but indeed that does not need to go into the lfric repos.

Additionally (not done atm since we didn't want to affect the rest of the lfric build), at this step we can ensure that psyclone_tools.py can be imported, meaning that cylc and interactive builds are identical (in this regard). And that's basically the same that happens in the make build: invoking make scripts from core will modify the PYTONPATH. We can also use this to move the handling of site-specific configurations out of the base class (since Matthew doesn't like that we modify sys.path there). This would mean, there is just one location that sets up python to work correctly everywhere.

FWIW, the current approach using local_build.py is to basically use a command line option to specify core (unless it is checked out ... yes, when we do a clean checkout we have the location, but the fab script we are designing is based on what happens once you have the two repos checked out, esp. to support interactive and cylc builds based on current paths). So something like:

$LFRIC_APPS/build/local_build.sh --core_source $LFRIC_CORE

Which is basically the same information :)

If we add a command line option to the fab scripts to indicate where the core sources are, we either:

1. Need an additional base class in lfric_apps (to avoid adding this command line option to all Fab scripts in lfric_apps) ... which imho still doesn't really solve the issue of how this new lfric_apps_base class can import the lfric_base class in the beginning (unless more messing around with sys.path in the script before importing of course :) ).
2. Add this to each individual fab script (useless code duplication, even if we import this from a common class)
3. use env variables??

Of, we could use an initialisation script (. $LFRIC_CORE/init_python?) and then just run it, but I don't find that any clearer.

All in all, I actually do like our solution (no surprise :) obviously I am biased ;) ), but am happy for other solutions. Suggestions welcome.

[hiker]

@MatthewHambley , @yaswant , @t00sa , @Pierre-siddall , @stevemullerworth , @cameronbateman-mo

We agreed in the telco on Thursdayto add a new base class to the apps repo, which is the base class for all lfric_apps scripts, and is responsible for modifying Python's sys.path to be able to import the base class of the core repository using a command line option. I have been trying to find various approaches, and tbh, all of them have disadvantages (including that @MatthewHambley might the solution still consider the solution to be using a wrapper script). Here what I have tried:

1. Adding an LFRicAppBase class is somewhat forward. Main issue is that the LFRicAppsBase class needs to import the LFRicBase class from core (which will be renamed to LFRicCoreBase of course). But as far as I can see it will trigger a flake8 error. Here my approach:

import sys
# Parse command line for --core command line option to find the location of lfric_core
lfric_core = args.core     # whatever
sys.path.append(lfric_core)

from lfric_base import LFRicBase    # requires the above modification of sys.path

class LFRicAppsBase(LFRicBase):

This triggers module level import not at top of file from flake8. I believe we could work around the linter warning by using importlib ... but that basically adds a lot of 'stuffing around' to work around a linter error (which contradicts the whole idea of a linter making the code more readable imho). Disable the linter warning?

Or am I missing a better solution here? Feedback really appreciated.

2. The next problem I have is: how can an application script find the LFRicAppsBase class? The apps base class would be in ../../build/lfric_apps_base ... but I can't use relative imports (ImportError: attempted relative import with no known parent package). I am not 100% familiar with pythonb package management, but adding __init__.py files doesn't help. We would have to turn lfric_apps into a proper python package in order to do that (then we could use from lfric_apps.build.lfric_apps_base import LFRicAppsBase)? I found some references to less known double-underscore methods (__module__), but that feels all just wrong.

Scott Wales had a different approach, using a top-level cli.py script, which would take the class to be called for building as paramter, e.g.:

../../cli.py --core path_to_lfric_core gungho ...

But I assume you would consider this as a wrapper script as well?

Could someone give me some details how you want this implemented? I.e. a set of dummy files, e,g.:

• lfric_core/lfric_core_base.py # containing LFRicCoreBase
• lfric_apps/build/lfric_apps_base.py # class inheriting from LFRicCoreBase
• lfric_apps/applications/gungho/fab_gungho.py # class inheriting from LFRicAppsBase

so that you can use ./fab_gungho.py to work. I think once someone explains to me how to do this, I can sort out the rest.

Thanks!