areusch commented on code in PR #12521: URL: https://github.com/apache/tvm/pull/12521#discussion_r951879892
########## python/tvm/autotvm/task/dispatcher.py: ########## @@ -238,12 +246,12 @@ class ApplyHistoryBest(DispatchContext): Parameters ---------- - records : str, list of str, or iterator of (autotvm.measure.MeasureInput,\ - autotvm.measure.MeasureResult) - Collection of tuning records. - If is str, then it should be the filename of a records log file. - Each row of this file is an encoded record pair. If it is a list, it can either be - a list of paths to log files that will be loaded jointly or an iterator or records. + records : Records or iterator of Records objects, where a Records + object is a path-like object, a file-like object, or an + iterator of (MeasureInput, MeasureResult). + + Collection of tuning records. If multiple Records objects are passed, their + contents will be merged. """ def __init__(self, records): Review Comment: should this arg get typed too? ########## python/tvm/autotvm/task/dispatcher.py: ########## @@ -30,18 +30,26 @@ from __future__ import absolute_import as _abs +from io import TextIOBase import logging -import typing -from typing import Union -from collections.abc import Iterable +from os import PathLike +from pathlib import Path +from typing import List, Iterable, Tuple, Union import numpy as np from .space import FallbackConfigEntity from .. import env as _env +from ..measure import MeasureInput, MeasureResult logger = logging.getLogger("autotvm") +Records = Union[ + Union[str, bytes, Path], # Path-like objects + TextIOBase, # File-like objects + Iterable[Tuple[MeasureInput, MeasureResult]], Review Comment: is this use case supported currently? i'm not sure whether we should support bypassing the load path here (this is technically "defensive programming" which i recognize is not great but i also think it might be prudent here). ########## python/tvm/autotvm/task/dispatcher.py: ########## @@ -458,11 +468,16 @@ def __init__(self, records): Otherwise, it is an iterator. """ # pylint: disable=import-outside-toplevel - from ..record import load_from_file + from ..record import load_from_file, load_from_buffer super(ApplyGraphBest, self).__init__() - if isinstance(records, str): + if isinstance(records, str, bytes, PathLike): Review Comment: i thought the types were supposed to be in a tuple, like: ```suggestion if isinstance(records, (str, bytes, PathLike)): ``` ########## python/tvm/autotvm/task/dispatcher.py: ########## @@ -238,12 +246,12 @@ class ApplyHistoryBest(DispatchContext): Parameters ---------- - records : str, list of str, or iterator of (autotvm.measure.MeasureInput,\ - autotvm.measure.MeasureResult) - Collection of tuning records. - If is str, then it should be the filename of a records log file. - Each row of this file is an encoded record pair. If it is a list, it can either be - a list of paths to log files that will be loaded jointly or an iterator or records. + records : Records or iterator of Records objects, where a Records Review Comment: i think it's reasonable to allow for the variety of ways to read in records, but just wondering if there's a use case for supporting `Iterator[Records]`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org