storm icon indicating copy to clipboard operation
storm copied to clipboard

feat(lm): Improved Modularity and Maintainability for LLM Integration

Open rmcc3 opened this issue 1 year ago • 0 comments

This pull request introduces a significant refactor of how LLMs are integrated into the STORM framework.

⚠️ Further testing is needed for other LLMs before a merge is considered. Please read Next Steps.

Justification:

The previous structure, where all LLM classes were defined within a single lm.py file, was becoming increasingly difficult to manage as more LLM providers were added. This monolithic approach hindered code readability, maintainability, and scalability.

This refactor addresses these issues by:

  • Enhancing Modularity: Organizing LLM classes into a dedicated providers subpackage promotes a more modular design. Each provider (OpenAI, Anthropic, DeepSeek, etc.) has its own file, making it easier to locate and maintain code related to specific LLMs.
  • Improving Scalability: The new structure facilitates easier integration of future LLM providers without cluttering the existing codebase.
  • Increasing Readability: Separating the LLM implementations into distinct files improves code readability and makes it easier for developers to understand the different LLM integrations.

Changes Made:

  1. New Provider Subpackage: Created a new directory structure under knowledge_storm/lm/providers, with each LLM class residing in its own file.
  2. File Renaming: Renamed lm.py to llm_base.py to resolve an import conflict. Imports in other files have been updated accordingly.
  3. Backwards Compatibility: Preserved backwards compatibility through reassignments in llm_base.py to prevent breaking existing code. However, importing directly from llm_base.py should now be considered deprecated.
  4. Testing: Only the DeepSeek model has been tested due to limited API access. DeepSeek ran three consecutive times without issue.

Next Steps:

  • Comprehensive Testing: As I do not have access to the other APIs, further testing of the refactored code with different LLM providers is required. However, I do not think there would be any issues.

Please let me know if any changes are needed.

rmcc3 avatar Jul 19 '24 19:07 rmcc3