theochem / cgrid

C++ version of horton (2.x) grid functionality
GNU General Public License v3.0
4 stars 1 forks source link

Build speed #16

Closed matt-chan closed 7 years ago

codecov[bot] commented 7 years ago

Codecov Report

Merging #16 into master will decrease coverage by 1.77%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #16      +/-   ##
==========================================
- Coverage     100%   98.22%   -1.78%     
==========================================
  Files           2        6       +4     
  Lines           3      225     +222     
  Branches        0       30      +30     
==========================================
+ Hits            3      221     +218     
- Partials        0        4       +4
Impacted Files Coverage Δ
qcgrids/cellgrid.cpp 95.23% <0%> (ø)
qcgrids/tests/common.cpp 100% <0%> (ø)
qcgrids/cellgrid.h 100% <0%> (ø)
qcgrids/tests/test_supergrid.cpp 98.75% <0%> (ø)

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 aa62576...6d924f4. Read the comment docs.

tovrstra commented 7 years ago

Is the pip cache no good?

tovrstra commented 7 years ago

Oh I see, there is no need for pip cache because they are installed in the conda env, which is already cached. That may be a source of trouble, because these will never get uninstalled, so we are stuck with the versions that were installed on the first time the cache was stored.

It would be better to install the pip package with --user, to avoid that these end up in the conda env. Caching may make sense then. In that case, caching pip as explained here is certainly useful: https://github.com/nickstenning/travis-pip-cache

matt-chan commented 7 years ago

Hmm you're right. The reason I turned off the pip cache was because pip has a http request cache which it invalidates fairly often, which makes travis repack the entire cache directory (which takes ~70s). The savings of pip caching aren't that high so it wasn't worth it.

I guess we could install everything from scratch each time. That seems really painful though. Do you think we can just delete the caches when we make changes to the pip install? It won't be so often, but we just have to remember...

Matt On Sat, 2 Sep 2017 at 22:22 Toon Verstraelen notifications@github.com wrote:

Oh I see, there is no need for pip cache because they are installed in the conda env, which is already cached. That may be a source of trouble, because these will never get uninstalled, so we are stuck with the versions that were installed on the first time the cache was stored.

It would be better to install the pip package with --user, to avoid that these end up in the conda env. Caching may make sense then. In that case, caching pip as explained here is certainly useful: https://github.com/nickstenning/travis-pip-cache

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theochem/qcgrids/pull/16#issuecomment-326767405, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_-NfN-_0kV2UlCIkEbZvcHQg56o_2oks5sebkIgaJpZM4PK6Bb .

-- Matt

Sent from my phone

tovrstra commented 7 years ago

It is not only when we change the pip install line, but also when new versions of the packages are released. That happens fairly often too.

Why does it take so long to tar the cache directory? Is this due to the size of the conda install?

tovrstra commented 7 years ago

Also conda packages update fairly frequently, which will also triger a repack. Does it really matter? I believe the cache is only updated when running on the master branch, not in a PR. See https://docs.travis-ci.com/user/caching/

matt-chan commented 7 years ago

Yeah, we should be running pip with --upgrade. I think we only run into a problem with the cache if we want to remove a package from pip (by deleting an entry in the pip install line). We should run into the same problem with conda packages too actually. I'm okay with having to build the cache every time there's an update. It's probably only once a day or so across all the packages combined.

No, the cache is definitely updated in a PR as well. There are caches on the feature branches and the master branches as well. Not sure which it uses in a PR though. It takes so long because it needs to get compressed and uploaded I guess? It's 500mb or so.

On Sat, 2 Sep 2017 at 22:59 Toon Verstraelen notifications@github.com wrote:

Also conda packages update fairly frequently, which will also triger a repack. Does it really matter? I believe the cache is only updated when running on the master branch, not in a PR. See https://docs.travis-ci.com/user/caching/

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/theochem/qcgrids/pull/16#issuecomment-326769081, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_-NceHE3KjjHx3oq_tZHHDIbDxJt3Cks5secGWgaJpZM4PK6Bb .

-- Matt

Sent from my phone

tovrstra commented 7 years ago

I'm merging this. dependencies for the debug build should improve still. I'll open a few issues with remaining things to be looked into.