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

Experiments resource-based interface #106

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

acroz
Copy link
Member

@acroz acroz commented May 10, 2019

This PR prototypes adding a resource-based interface for experiment runs, with the following features:

  • New primary models using attrs with class methods for querying / getting resources
  • Convert client models to 'resource interface' models
  • A query return type that behaves as an iterable but can provide extra methods like as_dataframe for other view types

Notes:

  • I discussed with @zblz about using dataclasses but we agreed that attrs was a reasonable substitute allowing us to maintain Python 2.7 support.
  • I've not done anything clever with any of:
    • Caching the session / client
    • Looking up the project ID when not set
  • Only part of the run objects are currently mapped to df columns
  • What are the columns in the output df called when metrics / params / tags have conflicting names?

@acroz
Copy link
Member Author

acroz commented May 10, 2019

Example usage:

from faculty.experiments import ExperimentRun

print(
    list(
        ExperimentRun.query(project_id="f20f5eaa-9cff-4216-8ebd-b88fd294bb3e")
    )
)

print(
    ExperimentRun.query(
        project_id="f20f5eaa-9cff-4216-8ebd-b88fd294bb3e"
    ).as_dataframe()
)

@haileyfong haileyfong force-pushed the experiments-resource-interface branch from 77bcd39 to 5ae2d09 Compare May 14, 2019 16:50
@eliasbenussi eliasbenussi force-pushed the experiments-resource-interface branch 2 times, most recently from b8ac2f3 to 5c063a9 Compare May 29, 2019 15:46
@eliasbenussi
Copy link
Contributor

  • Caching the session / client

I talked to @zblz about this. In faculty.client we call faculty.session.get_session which already handles caching of the session. I think it's not worth the effort to implement caching just for the client, so I would keep the implementation as is?

  • Looking up the project ID when not set

Sorry my confusion; by this you mean using the PROJECT_ID env variable by default if an argument is not passed?

  • What are the columns in the output df called when metrics / params / tags have conflicting names?

I am not very experienced with pandas, my first thought was to namespace keys by metrics. / params. / tags., but I don't know if this would be a nuisance in practice?

@eliasbenussi eliasbenussi force-pushed the experiments-resource-interface branch from 3a605a9 to 2d4050c Compare June 3, 2019 10:27
@eliasbenussi
Copy link
Contributor

The checks are not passing because apparently yield from in faculty/experiments.py (line 62) is not valid Python 2 syntax. I am not really sure how to address this?

@eliasbenussi eliasbenussi force-pushed the experiments-resource-interface branch from 2d4050c to 2955886 Compare June 3, 2019 13:26
@acroz acroz force-pushed the experiments-resource-interface branch 4 times, most recently from 72a6129 to bc3fac6 Compare June 19, 2019 10:41
@acroz acroz marked this pull request as ready for review June 19, 2019 10:41
@acroz acroz changed the title WIP: Resource-based interface Experiments resource-based interface Jun 19, 2019
@acroz acroz requested review from zblz, eliasbenussi and pbugnion June 19, 2019 10:44
@acroz acroz force-pushed the experiments-resource-interface branch from bc3fac6 to b41a9c9 Compare June 21, 2019 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants