linkedin / linkedin/Liger-Kernel
[RFC] More robust revert functions for convergence tests
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 6.6k
- Forks
- 603
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 47
Description
### 🐛 Describe the bug
Many discussions show that the current revert functions have several limitations, including:
- incomplete revert: https://github.com/linkedin/Liger-Kernel/pull/627#issuecomment-2757281103 #542
- not automatically updating old reference: #385
- comparing wrong references: https://github.com/linkedin/Liger-Kernel/pull/627#issuecomment-2756914629
Simply reloading the modules would generate new class/function objects different from the original ones, i.e. `revert(patch(object)) != object`, which can easily lead to bugs when doing comparisons (`isinstance()`), repatching, reusing configs and so on.
One way to address the issue is having a patch manager for testing to keep track of replaced objects, so we can easily revert back to the original objects after monkey patch. Or is there a way to achieve it with pytest?
cc @tyler-romero @austin362667
### Reproduce
_No response_
### Versions
none
Contributor guide
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 with the current revert functions used by the convergence tests, then review the linked discussions in PR #627 and issues #542 and #385. Define a testing approach that preserves original object identity, updates old references, and compares the correct references; done means these listed limitations are covered by reproducible tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100