[PATCH] D142168: [mlgo] Stream the training data

[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/TrainingLogger.h:122
 
-  // Warning! For int32_t, the return is set up for int64_t, so the caller needs
-  // to piecemeal cast their int32_t values.
-  // FIXME: let's drop int32_t support. While it's supported by evaluator, it's
-  // not supported by the tensorflow::SequenceExample proto. For small values,
-  // we can consider using bytes.
-  char *addEntryAndGetFloatOrInt64Buffer(size_t FeatureID);
+  std::optional<size_t> currentObservationID() const {
+    auto I = ObservationIDs.find(CurrentContext);
----------------
kazu wrote:
> If you only care about `currentObservationID().has_value()`, I wonder if you can rename this function to `isObservationInProgress` or something and do:
> 
> ```
>   return ObservationIDs.count(CurrentContext);
> ```
> 
good point, done!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142168