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

Associate Experiment with all models and matrices [Resolves #411] #476

Merged
merged 3 commits into from
Oct 30, 2018

Conversation

thcrock
Copy link
Contributor

@thcrock thcrock commented Oct 19, 2018

To aid in the lookup of relevant matrices and models to an experiment
afterwards, we want to record whether or not that experiment used that
matrix or model, regardless of whether or not it had to build it.

We still want to record which experiment built the matrix or model. This
was already being recorded in the models.experiment_hash column, which
is renamed to built_by_experiment. The matrices table gains a similar
column to mirror this functionality.

Audition is among the use cases for this change, so the pre-audition query is
updated to reflect this.

  • Create experiment_matrices and experiment_models tables
  • Rename models.experiment_hash to models.built_by_experiment to contain
    what is currently stored there but with a more specific name
  • Add matrices.built_by_experiment to mirror the models column.
  • Add utils to record these rows and trigger them from the Experiment
    experiment_models
  • Update preaudition query to look at experiment_models
    fixing
  • Add note on these new tables to Experiment Algorithm doc and
    Experiment Running doc
  • Update getcwd patch in audition tests to only exist where it needs to,
    fixing a local test failure where the getcwd was screwing up
    testing.postgresql

To aid in the lookup of relevant matrices and models to an experiment
afterwards, we want to record whether or not that experiment *used* that
matrix or model, regardless of whether or not it had to *build* it.

We still want to record which experiment built the matrix or model. This
was already being recorded in the models.experiment_hash column, which
is renamed to built_by_experiment. The matrices table gains a similar
column to mirror this functionality.

Audition is among the use cases for this change, so the pre-audition query is
updated to reflect this.

- Create experiment_matrices and experiment_models tables
- Rename models.experiment_hash to models.built_by_experiment to contain
what is currently stored there but with a more specific name
- Add matrices.built_by_experiment to mirror the models column.
- Add utils to record these rows and trigger them from the Experiment
experiment_models
- Update preaudition query to look at experiment_models
fixing
- Add note on these new tables to Experiment Algorithm doc and
Experiment Running doc
- Update getcwd patch in audition tests to only exist where it needs to,
fixing a local test failure where the getcwd was screwing up
testing.postgresql
@codecov-io
Copy link

codecov-io commented Oct 19, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@bede4d0). Click here to learn what that means.
The diff coverage is 64.81%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #476   +/-   ##
=========================================
  Coverage          ?   82.76%           
=========================================
  Files             ?       81           
  Lines             ?     4643           
  Branches          ?        0           
=========================================
  Hits              ?     3843           
  Misses            ?      800           
  Partials          ?        0
Impacted Files Coverage Δ
src/triage/component/catwalk/model_trainers.py 94.48% <ø> (ø)
src/triage/component/results_schema/__init__.py 64.86% <ø> (ø)
src/triage/component/audition/pre_audition.py 100% <ø> (ø)
...7d013686_associate_experiments_with_models_and_.py 0% <0%> (ø)
src/triage/component/catwalk/utils.py 98.75% <100%> (ø)
src/triage/experiments/base.py 92.89% <100%> (ø)
src/triage/component/architect/builders.py 88.63% <100%> (ø)
src/triage/component/results_schema/schema.py 97.6% <100%> (ø)
src/triage/component/architect/planner.py 96.22% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bede4d0...f05211f. Read the comment docs.

Copy link
Member

@jesteria jesteria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool 👍

src/tests/test_experiments.py Outdated Show resolved Hide resolved
src/triage/component/results_schema/schema.py Show resolved Hide resolved
Copy link
Contributor

@tweddielin tweddielin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. It's pretty cool now for experiments not building matrices or models but just using them are gonna be recorded. Looks good to me. Just the commented out experiment thing.

src/tests/audition_tests/test_preaudition.py Outdated Show resolved Hide resolved
@thcrock thcrock merged commit 4a62242 into master Oct 30, 2018
@thcrock thcrock deleted the new_tables branch October 30, 2018 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants