Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

Remove local witness generation to align codebase and tests with production usage #23

Merged
merged 7 commits into from
Jan 26, 2024

Conversation

mcarilli
Copy link
Collaborator

@mcarilli mcarilli commented Jan 11, 2024

What ❔

At a high level, the prover can run in four possible configs: (external witness synthesis, local synthesis) x (all gpu memory, limited memory). In production we use (external, all).

The local-synthesis path adds complex code that we don't really need. After discussion with @robik75 we decided to remove it, pending @saitima's approval.

Why ❔

Removing local synthesis simplifies the codebase, the tests, and Robert's WIP caching improvements.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via cargo fmt and cargo lint.

@mcarilli mcarilli requested review from saitima and robik75 January 11, 2024 05:41
@mcarilli mcarilli marked this pull request as draft January 11, 2024 05:42
@robik75
Copy link
Member

robik75 commented Jan 11, 2024

Production does not use "limited" memory.

saitima
saitima previously approved these changes Jan 16, 2024
@mcarilli mcarilli changed the title [WIP] Improves alignment of unit tests with production usage [WIP] Align codebase and tests with production usage Jan 17, 2024
@mcarilli mcarilli changed the title [WIP] Align codebase and tests with production usage Align codebase and tests with production usage Jan 18, 2024
@mcarilli mcarilli marked this pull request as ready for review January 18, 2024 02:51
@mcarilli mcarilli changed the title Align codebase and tests with production usage Remove local witness generation to align codebase and tests with production usage Jan 20, 2024
Copy link
Member

@saitima saitima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good to me if all tests are successfull.

@mcarilli mcarilli merged commit 67f06dc into main Jan 26, 2024
4 checks passed
@robik75 robik75 deleted the mc-test-like-prod branch August 6, 2024 12:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants