Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flexible base reporters for file IO, web, and DBs #48

Open
nicholasjng opened this issue Feb 1, 2024 · 3 comments · May be fixed by #74
Open

Add flexible base reporters for file IO, web, and DBs #48

nicholasjng opened this issue Feb 1, 2024 · 3 comments · May be fixed by #74
Assignees
Labels
enhancement New feature or request

Comments

@nicholasjng
Copy link
Collaborator

Each of these should inherit directly from BenchmarkReporter and define a nice interface for the respective IO to be reusable or even useable out of the box for most users.

Pseudocode example:

class FileIOReporter:
    def write_record(self, r):
        ...

    def write_record_batched(self, rb):
        ...

    def read_record(self):
        ...
    
    def open(self, fp):
        ...

    def close(self, fp):
        ...


class JSONFileReporter(FileIOReporter):

    def open(self, fp):
        with open(fp, "r") as f:
            return json.load(f)

    def close(self, fp):
        fp.close()


class YAMLFileReporter(FileIOReporter):
    ... # same thing as for JSON, but use YAML read/write APIs.

Objective: Implement the base BenchmarkReporter's interface for a few specific families of reporters, not every reporter by itself.

Bonus: Maybe the IO (open, close) can even become a mixin that makes specific file or database reporters trivial by just acquiring and releasing a handle to write to.

@nicholasjng nicholasjng added the enhancement New feature or request label Feb 1, 2024
@Hrsj123
Copy link

Hrsj123 commented Feb 6, 2024

I was wondering what is the advantage of having def read_record() method? Does it have any specific use?

@nicholasjng
Copy link
Collaborator Author

nicholasjng commented Feb 6, 2024

Thanks for the question. This is good for fetching historical data that you've already saved, for example like this (pseudocode):

import nnbench

runner = nnbench.BenchmarkRunner()
res = runner.run(...)

reporter = MyReporter()
historical = reporter.read_record("earlier.json")  # <- this needs dynamic arguments, like a filename in the flatfile case, or a query in the database case.
reporter.report(historical, res, ...)

Is this sensible to you? None of this is set in stone yet, so I'm happy to take suggestions.

@Hrsj123
Copy link

Hrsj123 commented Feb 7, 2024

Thanks for your clarification, it makes sense.

@Hrsj123 Hrsj123 linked a pull request Feb 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants