Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(lightrag): 优化hashing_kv初始化逻辑 #477

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

theClassLight
Copy link
Contributor

背景:

  1. enable_llm_cache=False时,llm_response_cache 赋值为None
  2. 在lightrag.py中,hashing_kv在原代码中直接赋值为llm_response_cache ,直接导致所有依赖hashing_kv传值的对象报错
  3. 所有查询依赖于llm.py中的xxx_model_complete方法,而方法需要hashing_kv获取model_name,最终导致恶性bug
  4. 考虑到hashing_kv的使用,选择了修改hashing_kv的初始化逻辑实现改动最小化
  • 修改了 llm_model_func 和 query 方法中的hashing_kv初始化逻辑
  • 当 self.llm_response_cache 不存在或没有 global_config 属性时,会创建一个新的hashing_kv实例
  • 所有基于hashing_kv获取global_config属性的对象可以正常运行

Background:

  1. If enable_llm_cache=False, llm_response_cache is set to None
  2. In lightra.py, hashing_kv is directly assigned to llm_response_cache in the original code, which directly causes all objects that rely on hashing_kv to report an error
  3. All queries rely on the xxx_model_complete method in llm.py, which requires hashing_kv to get model_name, resulting in a vicious bug
  4. Considering the use of hashing_kv, chose to modify the initialization logic of hashing_kv to minimize changes
  • Changed the hashing_kv initialization logic in the llm_model_func and query methods
  • When self.llm_response_cache does not exist or there is no global_config attribute, a new hashing_kv instance is created
  • All objects that obtain the global_config attribute based on hashing_kv can run properly

- 修改了 llm_model_func 和 query 方法中的hashing_kv初始化逻辑
- 当 self.llm_response_cache 不存在或没有 global_config 属性时,会创建一个新的hashing_kv实例
- 所有基于hashing_kv获取global_config属性的对象可以正常运行
@LarFii LarFii merged commit c840a05 into HKUDS:main Dec 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants