[PATCH] D89626: [ML] Add final reward logging facility.

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin marked an inline comment as done.
mtrofin added inline comments.


================
Comment at: llvm/include/llvm/Analysis/Utils/TFUtils.h:142-145
+  template <typename T> void logFinalReward(T Value) {
+    assert(RawLogData.back().empty());
+    logReward(Value);
+  }
----------------
yundiqian wrote:
> It seems over-complicated to pass the flag FinalReward to writeRawTensorsAsFeatureLists function and treat the case separately. 
> 
> How about making the RawLogData ready-to-print (reward vector is <0, 0, ..., 0, reward>) so that we don't need to change writeRawTensorsAsFeatureLists function? basically the user is supposed to make sure the data in RawLogData is ready-to-print and writeRawTensorsAsFeatureLists only takes care of printing format.
> 
> We can either:
> 1. logReward(0) in each step
> 2. have a function in Logger called logFinalReward(T value) or overwriteFinalReward(T value), which overwrites the value in RawLogData.back()->back()
> 
> or:
> 1. don't logReward(0) in each step
> 2. have a function in Logger called logFinalReward(T value) that fills in 0 in RawLogData.back() except putting reward in the last, we can tell the length by looking at feature length already logged in RawLogData
That would mean keeping around a potentially large array containing 0 (except the last value).

I would rather not add memory overhead if it can be avoided, and the extra consideration in code isn't that hard to grok.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89626/new/

https://reviews.llvm.org/D89626