Macaulay2 / Macaulay2/M2

Increasing memory limits & reducing memory usage

Open
#1,890 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Core Memory Leak
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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.