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

Fix #650 and #651 #652

Merged
merged 36 commits into from
Jan 5, 2019
Merged

Fix #650 and #651 #652

merged 36 commits into from
Jan 5, 2019

Conversation

wlandau
Copy link
Member

@wlandau wlandau commented Jan 5, 2019

Summary

  1. Pre-compute 3 graphs:
    1. config$graph: has everything in it, including output files.
    2. config$imports: a graph of just the imports. They do their own thing.
    3. config$schedule: a malleable schedule of targets. Excludes the imports, file outputs, etc. This is the graph we will try to modify while targets are building in DSL based on dplyr-like verbs? #233.
  2. For each target, pre-compute a list of the dependencies that need to be (re)loaded into memory before building, triggers, etc.
  3. Get rid of the attempt flag, reduces reads/writes to the cache.

All this could potentially increase speed, though I have not done any profiling yet.

Related GitHub issues and pull requests

Checklist

  • I have read drake's code of conduct, and I agree to follow its rules.
  • I have listed any substantial changes in the development news.
  • I have added testthat unit tests to tests/testthat to confirm that any new features or functionality work correctly.
  • I have tested this pull request locally with devtools::check()
  • This pull request is ready for review.
  • I think this pull request is ready to merge.

@codecov-io
Copy link

codecov-io commented Jan 5, 2019

Codecov Report

Merging #652 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #652   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          71     70    -1     
  Lines        5482   5453   -29     
=====================================
- Hits         5482   5453   -29
Impacted Files Coverage Δ
R/clean.R 100% <ø> (ø) ⬆️
R/loop.R 100% <ø> (ø) ⬆️
R/dependencies.R 100% <ø> (ø) ⬆️
R/cache_namespaces.R 100% <ø> (ø) ⬆️
R/future.R 100% <ø> (ø) ⬆️
R/hash_table.R 100% <100%> (ø) ⬆️
R/checksums.R 100% <100%> (ø) ⬆️
R/graph.R 100% <100%> (ø)
R/triggers.R 100% <100%> (ø) ⬆️
R/check.R 100% <100%> (ø) ⬆️
... and 19 more

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 8d60f7e...0aafdd8. Read the comment docs.

@wlandau wlandau merged commit abc03a1 into master Jan 5, 2019
@wlandau wlandau deleted the schedule_experiment branch January 5, 2019 04:29
@wlandau wlandau mentioned this pull request Jan 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants