linkedin / linkedin/Liger-Kernel
`revert_liger_kernel_to_xxx` can't revert LigerCrossEntropyLoss for transformers>=4.46.1
- Dominant language
- Python
- Stars
- 6.6k
- Forks
- 603
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 47
Description
### 🐛 Describe the bug
#369 found that CrossEntropyLoss wasn't applied in post-grad-acc-fix versions of transformers. Despite the fact that #375 fixed the issue, it didn't consider the revert functions for convergence test.
Currently, the convergence test, `test_mini_models_with_logits`, is comparing two models which both are using LigerCrossEntropyLoss except the first test case. In other words, **the test results might be false positive** in the second and later test cases
The implementation of current revert functions is reloading module by calling `importlib.reload(module_name)`. We can fix the issue by carefully checking the transformers version and adding all patched modules for reloads. We should also enhance our monkey_patch unit test by adding another revert and compare, ensuring the correctness of convergence test results.
### Reproduce
Add a print statement in LigerCrossEntropyLossFunction and run
```
python3 -m pytest test/convergence/test_mini_models_with_logits.py -v -rP
```
### Versions
none
Contributor guide
Research direction
Start by running test/convergence/test_mini_models_with_logits.py with -v -rP and inspect the revert functions that reload modules via importlib.reload. Review the monkey-patch unit test and LigerCrossEntropyLossFunction; done means the convergence cases compare correctly after each revert, including transformers versions >=4.46.1.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- machine-learning, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100