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

try moving GC to a callback #687

Closed
wants to merge 1 commit into from
Closed

try moving GC to a callback #687

wants to merge 1 commit into from

Conversation

simonbyrne
Copy link
Member

PULL REQUEST

Purpose and Content

Disable the automatic GC, call it periodically via a callback.

Benefits and Risks

Mucking around with the GC is dangerous, but it might get us some better scaling. Need to tune how often we call it.

Linked Issues

PR Checklist

  • This PR has a corresponding issue OR is linked to an SDI.
  • I have followed CliMA's codebase contribution and style guidelines OR N/A.
  • I have followed CliMA's documentation policy.
  • I have checked all issues and PRs and I certify that this PR does not duplicate an open PR.
  • I linted my code on my local machine prior to submission OR N/A.
  • Unit tests are included OR N/A.
  • Code used in an integration test OR N/A.
  • All tests ran successfully on my local machine OR N/A.
  • All classes, modules, and function contain docstrings OR N/A.
  • Documentation has been added/updated OR N/A.

@bischtob
Copy link
Contributor

bischtob commented Sep 2, 2022

@simonbyrne, is this current or should be closed?

@simonbyrne simonbyrne closed this Oct 2, 2022
@simonbyrne simonbyrne deleted the sb/gc-callback branch October 2, 2022 21:54
@simonbyrne simonbyrne mentioned this pull request Oct 2, 2022
10 tasks
bors bot added a commit that referenced this pull request Oct 3, 2022
821: make GC deterministic in distributed r=simonbyrne a=simonbyrne

# PULL REQUEST

## Purpose and Content
This should reduce MPI Waitall time by manually triggering the GC across all processes at the same time.

## Benefits and Risks
The number of steps will require some tuning to avoid out-of-memory errors

## Linked Issues
- Item 3 of #635 
- Mentioned in #686
- Supersedes #687


## PR Checklist
- [x] This PR has a corresponding issue OR is linked to an SDI.
- [x] I have followed CliMA's codebase [contribution](https://clima.github.io/ClimateMachine.jl/latest/Contributing/) and [style](https://clima.github.io/ClimateMachine.jl/latest/DevDocs/CodeStyle/) guidelines OR N/A.
- [x] I have followed CliMA's [documentation policy](https://github.com/CliMA/policies/wiki/Documentation-Policy).
- [x] I have checked all issues and PRs and I certify that this PR does not duplicate an open PR.
- [x] I linted my code on my local machine prior to submission OR N/A.
- [x] Unit tests are included OR N/A.
- [x] Code used in an integration test OR N/A.
- [x] All tests ran successfully on my local machine OR N/A.
- [x] All classes, modules, and function contain docstrings OR N/A.
- [x] Documentation has been added/updated OR N/A.


Co-authored-by: Simon Byrne <[email protected]>
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