logix-project / logix

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

Extend Analog.eval #50

Closed hwijeen closed 9 months ago

hwijeen commented 9 months ago

https://github.com/sangkeun00/analog/pull/49#discussion_r1424128503

With this PR, the users don't deal with log, hessian, save.

sangkeun00 commented 9 months ago

Thanks! However, I see two issues here. First, I believe we shouldn't set log=["grad"] in eval.

  1. While our current focus is on influence function where you need to set log=["grad"], we still have other options like "forward" and "bacwkard". I intentionally avoided setting log in eval because I wanted to let users just keep using the same log config as in the last logging phase. If they need to change they can always manually update it with update.
  2. You many not need to pass data_id during the eval phase, but this may be needed if we want to save influence scores for (train, test) pairs. I wonder whether not having data_id in the official example would give a wrong impression that data_id is not needed in the eval phase. However, this is not a major issue.

Let me know what you think about the above issues.

hwijeen commented 9 months ago

Oh you're right. I was just considering IF case. Hmm I still feel like asking the new comers to do log=["grad"] will be error prone. Since IF is our representative use case for now, would it make sense to internally set log to the correct value for IF (not in .eval but in somewhere else)

sangkeun00 commented 9 months ago

We can set the default log value to ["grad"] in Config. However, as this is irrelevant to eval, I am closing this PR for now.