[PATCH] D139370: [mlgo] Dependency-free training mode logger

Mircea Trofin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 5 15:11:55 PST 2022


mtrofin added inline comments.


================
Comment at: llvm/lib/Analysis/models/log_reader.py:34
+  @classmethod
+  def from_dict(d: dict):
+    name = d['name']
----------------
jacobhegna wrote:
> this won't work, no? if it's a `classmethod` the first argument could be `cls`, and the point is you could return `cls(name=...)` instead of `TensorSpec(name=...)`, it's essentially the same but follows the convention I see people using for factory methods in python
(sorry, went to fast over it)

what does it give us, though, compared to the @staticmethod way?


================
Comment at: llvm/lib/Analysis/models/log_reader.py:50
+
+  def spec(self):
+    return self._spec
----------------
jacobhegna wrote:
> nit: return type annotations? same for next two methods
__getitem__ is a bit trickier I think because it's the value of the underlying type. doesn't seem pytype lets us specify `self._spec.type`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139370



More information about the llvm-commits mailing list