logix-project / logix

AI Logging for Interpretability and Explainability🔬
Apache License 2.0
74 stars 6 forks source link

add_lora() sets init_strategy="random" if using initialize_from_log() with state computed by PCA #74

Closed hage1005 closed 8 months ago

hage1005 commented 8 months ago

In lora.py

if self.init_strategy == "pca" and len(covariance_state) == 0:
      get_logger().warning(
          "Hessian state not provided. Using random initialization instead."
      )
      self.init_strategy = "random"

we set init_strategy = "random"` if covariance state is not provided. This should be avoided if it's called from initialize_from_log(), which might use PCA but doesn't have covariance.

sangkeun00 commented 8 months ago

This design was intentional. PCA vs random only matters for initializing LoRA weights. However, when you call initialize_from_log, LoRA initialization is directly handled by saved state_dict. Therefore, as long as we ensure that the rank structure is the same, it doesn't matter whether self.init_strategy is pca or random. Please take a look at the below code snippet for the detail.

https://github.com/sangkeun00/analog/blob/f127707f393092755cf1aba7c0a4daedf3b0b1ad/analog/analog.py#L322-L329

hage1005 commented 8 months ago

Yeah it not affecting the functionality and correctness. I was concerned that it might introduce potential issues for the future.