Increasing memory limits & reducing memory usage
Nobody has claimed this yet.
- Dominant language
- Macaulay2
- Stars
- 435
- Forks
- 297
- Avg merge
- 4d 20h
- Merged PRs (30d)
- 11
Description
As M2 grows, one would expect that the existing algorithms would become more time or memory efficient, but every now and then a test or example that _used to work_ starts running out of memory. The typical response to this has been removing the test/example or making it smaller. Take the example of https://github.com/Macaulay2/M2/commit/1c4e8e5494a3faf21d771ce0afad48ee620f4e08, where a test involving the twisted cubic, which is a very standard example, was reduced to a rational normal curve.
I think this is backwards: instead of erasing the problem, we should look into the cause of the regression. I can think of two possible reasons and corresponding solutions:
1. there's a new memory leak somewhere => find and patch it before merging
2. the M2-binary simply requires more memory => extend the 400MB memory limit
You'd be hard pressed to find a system with less than 4GB memory these days (unless you want a Raspberry Pi or something, but I don't think that's the target here). Even github actions runners have [7GB](https://docs.github.com/en/actions/reference/specifications-for-github-hosted-runners) memory, so I think the second option is very reasonable if we are certain that the issue isn't the first case.
Recent changes with `capture` have cut down the time it takes to install and check packages significantly. Additionally, we used to need 2 concurrent M2 processes, one running `check`, and one actually running the example, which required 2x memory usage. Now, with `capture`, for most tests we only need one M2 process, cutting memory usage by half. Given that, I think it's safe to even double the 400MB memory limit.
That said, it seems like using `capture` has revealed some memory leaks (see #1689 and https://github.com/Macaulay2/M2/issues/1708#issuecomment-748664810). While we work on finding those memory leaks and as a last measure, rather than erasing the problem we should quarantine the tests or examples, maybe in `tests/quarantine`, so we can regularly test to see if any of them have been resolved. This would also provide a quantitative measure for whether an optimization (e.g. patching a memory leak, a better gb algorithm, using flint integers, etc.) has actually fixed the problem or not.
## TODO:
- [ ] increase the memory
- [ ] collect all tests and examples which have been removed or simplified in `tests/quarantine`
- ideally, also include the commit where the test was changed, so it can be easily reverted
- [ ] use capture for detecting memory leaks in each tests and create issues for addressing them (see #1689)
Here a list of such changes, so they can be reverted, or added as benchmarks, eventually:
- [ ] 1c4e8e5494a3faf21d771ce0afad48ee620f4e08 (TateOnProducts)
- [ ] b3e971b3633ba5da016c2b8cf932cc3deab35054 (IntegralClosure, #1537)
- [ ] f4fafd3 (Points, #1838)
- [ ] 7d5aa2d (EigenSolver, #1633)
- [ ] 08984b98df3b79c80d634f4db347638b09dace0a (SparseResultants, #1915)
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the 400MB memory limit and the listed commits affecting TateOnProducts, IntegralClosure, Points, EigenSolver, and SparseResultants. Examine how capture is used for package checks and memory-leak detection, and review #1689. Done means the limit decision is implemented and affected tests or examples are collected in tests/quarantine with their source commits recorded.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions
- Domain
- ci-cd, performance, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100