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

an attempt to make compiling files faster #814

Merged
merged 13 commits into from Aug 5, 2019

Conversation

@anoukvlug
Copy link
Contributor

commented Jun 21, 2019

This PR is a draft of an attempt to speed up the compiling of climate files and run_output files.

Closes #751 & #752 & #837

  • Entry in whats-new.rst
@pep8speaks

This comment has been minimized.

Copy link

commented Jun 21, 2019

Hello @anoukvlug! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-08-05 20:18:43 UTC

anoukvlug added some commits Jul 1, 2019

@anoukvlug

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@fmaussion could you have a look at what you think of this PR?

@fmaussion

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

Sorry for being so late on this one - this is very good work.

I will make a couple of changes if you agree: I'd like to simply use your method as default (i.e. not add a new task). I'll push on your branch if that's ok?

@anoukvlug

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

No worries and yes, that is fine for me :)

@anoukvlug

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

FYI: I didn't optimize the size (lsubfiles) of the temporary files that are being generated. I put it now as 1000 glaciers per temporary file. I think 1000 should be a good number, however there is probably a number that is more optimal.

fmaussion added some commits Jul 31, 2019

@fmaussion fmaussion referenced this pull request Jul 31, 2019
@fmaussion

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

OK @anoukvlug - I finally spent some time on this.

I have added some "python complexity" by using a decorator, but basically the idea is the same as yours. We need dask installed for the tests to run, so I can't merge right away.

I also deprecated filesuffix in favor of input_filesuffix -> your code will still work, but OGGM will raise a warning.

@anoukvlug

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

Thanks @fmaussion for improving this PR!

fmaussion added some commits Aug 4, 2019

fmaussion added some commits Aug 5, 2019

@fmaussion fmaussion merged commit b9c3095 into OGGM:master Aug 5, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 86.888%
Details
@fmaussion

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

Thanks @anoukvlug ! And sorry this took so long...

@anoukvlug anoukvlug deleted the anoukvlug:compile_faster branch Aug 7, 2019

@anoukvlug

This comment has been minimized.

Copy link
Contributor Author

commented Aug 7, 2019

No worries and thank you!

@fmaussion

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

Let me know it you encounter any problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.