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

Add an executor abstraction #4

Merged
merged 7 commits into from
Oct 29, 2024
Merged

Add an executor abstraction #4

merged 7 commits into from
Oct 29, 2024

Conversation

lewisjared
Copy link
Contributor

@lewisjared lewisjared commented Oct 28, 2024

Description

Adds the Executor abstraction.
Currently we only have the local runner, but we will eventually need a way of running this out of process.

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

@lewisjared lewisjared requested a review from nocollier October 28, 2024 10:37
@lewisjared
Copy link
Contributor Author

@nocollier very basic but we keeps the door open for how we choose to execute metrics. The interface will be in flux until we figure out what running a metric actually looks like

@nocollier
Copy link

Thanks for tagging me, I see what you have in mind. My only comment is that you list 'subprocesses' as a future executor. If you are thinking the python package subprocesses, my understanding is that this will not do multi-node parallelism. In my experience we will need this because we will be pushing the memory of any computing resource if we try to run many of these metrics at once. Also, as the package scales up, it will take non-trivial time which can easily be circumvented if running on HPC systems and we can utilize multiple nodes.

mpi4py is the only way I have come across in python of doing multi-node parallelism, but perhaps I am not well informed. It does complicate installation, but we could always keep it in a optional installation a la pip install cmip-ref[mpi] or some such thing.

@lewisjared
Copy link
Contributor Author

Cool. I didn't have anything particular in mind. I've used celery for that in the past which worked well, but does add more moving parts.

I'm pretty sure whatever approach will need to use different virtual environments, which makes things a bit harder, but it would be easier to build that assumption in from the start.

@lewisjared lewisjared merged commit 43db470 into main Oct 29, 2024
12 checks passed
@lewisjared lewisjared deleted the executor branch October 29, 2024 09:10
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