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

Executable path in configuration branches #133

Closed
micaeljtoliveira opened this issue Apr 8, 2024 · 14 comments
Closed

Executable path in configuration branches #133

micaeljtoliveira opened this issue Apr 8, 2024 · 14 comments

Comments

@micaeljtoliveira
Copy link
Contributor

With the current setup, the configurations' executable path includes the corresponding spack hash. Although this is fine and desirable for releases (albeit not necessary, as it would be better to replace the hash by a more meaningful version number) this can be a major hurdle during development, as a PR needs to be made for all configurations to update the hash whenever we update the OM3 executables.

The current approach does have some important advantages:

  • it makes clear what was the latest executable that was used for a given configuration
  • having to open a PR to update each configuration allows/requires some testing, either manually, as done now, or automatically, once some proper CI is set for the configurations.
  • exec manifest is kept in sync with the used executable

On the other hand, the number of "official" configurations is going to increase in the short/medium term, making it quite cumbersome to update the executable over a dozen configurations, greatly increasing the amount of work required from the developers to keep things up-to-date. This can be mitigated once we have an automated cherry-pick mechanism in place, but it will still requite some work.

Instead, one could change the exec path to be always the same for the development branches. I did not propose this before, as initially I thought some changes in payu would be required, but in the meantime I've realised this can also be done using spack environment views. I think this approach would be fine for now, specially because frequent updates are to be expected at this stage in development. Any releases would still require the path to be updated. Later on, once development slows down, we can always revisit this choice.

@COSIMA/access-om3-developers Happy to get some feedback on this before I start creating some PRs.

@dougiesquire
Copy link
Collaborator

dougiesquire commented Apr 10, 2024

Would this mean that there will be no way to determine which exe was used for a specific commit of a configuration?

@micaeljtoliveira
Copy link
Contributor Author

Would this mean that there will be no way to determine which exe was used for a specific commit of a configuration?

If you are simply looking at the configuration git repo, no, not directly. You would still know it was a version of OM3 installed in the COSIMA Spack instance in the access-om3_0.x.0 environment (or whatever we decide to call the development sandbox).

@anton-seaice
Copy link
Contributor

  • having to open a PR to update each configuration allows/requires some testing, either manually, as done now, or automatically, once some proper CI is set for the configurations.

As always ... I think this is a key step that I don't want us to be skipping! :)

I also have the same questions as Dougie ... how do you know which executable works with which configuration when new options are added (or if we move to a different major release one day). Especially when we start keeping and analysing longer model runs etc.

@micaeljtoliveira
Copy link
Contributor Author

Sorry, maybe this was not very clear, but this would only be for development. When doing a release of a configuration one would update the OM3 exec path and payu manifest to point to a released version of OM3. Therefore, from the point of view of the users, it would always be clear what version of OM3 is required to run a given configuration.

Otherwise, the assumption would always be that the tip of configuration branches should work with the tip of the OM3 main branch. If it does not work, then it needs to be fixed ASAP anyway.

@micaeljtoliveira
Copy link
Contributor Author

PS:

Especially when we start keeping and analysing longer model runs etc.

I would say one should never use the development version of OM3 or the configurations for production, only properly released versions.

@dougiesquire
Copy link
Collaborator

Relevant: ACCESS-NRI/model-config-tests#10

@micaeljtoliveira
Copy link
Contributor Author

Relevant: ACCESS-NRI/model-config-tests#10

@dougiesquire Thanks for linking the issues. That actually helps in clarifying things. In particular, there is a key difference between what is discussed here and what is discussed in ACCESS-NRI/model-config-tests#10

Each release goes to its own spack environment and these are (hopefully) never removed/changed, while there is only one development spack environment (currently called access-om3-0_x_0) and whenever we update the environment we "delete" the previous installation.

That means that in the released configurations, the path to the environment where OM3 is installed must be specified somehow, either through the modules, through the exec path, or both. What I suggested some time ago and @aidanheerdegen is now pushing forward is to only specify it through the modules.

On the other hand, when using the spack development environment, one cannot use the modules, as these do not change when updating OM3, only when updating spack itself. This can still be done with the exec path, as that currently contains a unique spack generated hash that changes whenever changes are done to the spack environment.

What I'm suggesting in this issue here is to actually never bother distinguishing the OM3 executables used in the development versions of the configurations and keep the exec path fixed.

@aekiss
Copy link
Contributor

aekiss commented Apr 11, 2024

  • having to open a PR to update each configuration allows/requires some testing, either manually, as done now, or automatically, once some proper CI is set for the configurations.

As always ... I think this is a key step that I don't want us to be skipping! :)

This seems a key capability to have - otherwise an exe update could break development configs without us realising it, right? To avoid that we'd still need to check every config branch somehow, so it doesn't seem any less onerous to me.

Should we prioritise setting up automated cherry-picking (#119) instead? That would help automate checking that the configs all work with the new exe.

@micaeljtoliveira
Copy link
Contributor Author

Summary of recent meeting where this issue was discussed:

There are basically 4 options for the exec path:

  1. the development tip of OM3, using a generic path
  2. an hardcoded development version of OM3
  3. a released version of OM3
  4. an NRI deployed version of OM3

Option 2. was the favoured one, in order to ensure that executables being used by developers never change silently.

@micaeljtoliveira
Copy link
Contributor Author

Today I spent some time to figure out how to get the git commit in the exec path, so we can have multiple development versions installed simultaneously, and the simplest is to provide spack with the specific commit to be used when installing OM3. For example:

[email protected]

This works fine (after solving a couple of issues with submodules) and we end up with the following exec path:

/g/data/ik11/spack/0.21.2/opt/linux-rocky8-cascadelake/intel-2021.10.0/access-om3-d6813d6b9e1df560ac3f6ba6a605daab9cfd9569_main-5pjh7z2/bin/access-om3-MOM6-CICE6

A bit long, as it includes the full git hash plus a truncated spack hash, but does what we want.

@dougiesquire
Copy link
Collaborator

This is great - thanks @micaeljtoliveira

@aekiss
Copy link
Contributor

aekiss commented May 16, 2024

LGTM, thanks @micaeljtoliveira - can this issue be closed now?

@micaeljtoliveira
Copy link
Contributor Author

I haven't checked if this has now been implemented in all the configs. If that's the case, then yes, this issue should be closed.

@dougiesquire
Copy link
Collaborator

dougiesquire commented May 17, 2024

I'll do the missing one now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants