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 download of EEZ to happen on local (login) node #141

Merged

Conversation

timtroendle
Copy link
Member

@timtroendle timtroendle commented Jul 22, 2021

Fixes an issue introduced in #99.

Checklist

Any checks which are not relevant to the PR can be pre-checked by the PR creator. All others should be checked by the reviewer. You can add extra checklist items here if required by the PR.

  • CHANGELOG updated
  • Minimal workflow tests pass
  • Tests added to cover contribution
  • Documentation updated
  • Configuration schema updated

@timtroendle timtroendle added the bug Something isn't working label Jul 22, 2021
@timtroendle timtroendle added this to the 1.1 milestone Jul 22, 2021
Copy link
Member

@brynpickering brynpickering left a comment

Choose a reason for hiding this comment

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

Hmm, this has never been a problem for me. Do you get errors when using job nodes for downloads or is this just your preference? I just wonder whether it's a good design choice to have downloads on the login node. Parallelising them would speed things up.

@timtroendle
Copy link
Member Author

Yes, this fails in my setup as compute nodes have no internet access. I believe you said once it's possible to change that, but I'd rather not if not necessary.

It's true that downloads on compute nodes could be faster as they could be parallelised. But you can also parallelise downloads on the login node using --local-cores = [number greater 1].

In any case, all download rules right now are local. So we should be consistent either way.

@@ -14,6 +14,7 @@ SCHEMA_UNITS = {

configfile: "config/default.yaml"
localrules: download_raw_gadm_administrative_borders, raw_gadm_administrative_borders, download_raw_nuts_units
localrules: download_eez
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this is on a new line? I would find it easier to understand if the first line were to become a multi-line listing of localrules.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm the first line is too long. There may be other ways of splitting this into several lines with just a single localrules call, but I am not exactly sure how.

@brynpickering
Copy link
Member

OK, would be good to think of a section in the documentation on how best to leverage snakemake args in Euro-Calliope (since their documentation is difficult to navigate, I find). Things like --local-cores could then be mentioned there.

@timtroendle
Copy link
Member Author

Good idea. I've opened issue #144 for that.

@timtroendle timtroendle merged commit 42526ea into calliope-project:develop Jul 22, 2021
@timtroendle timtroendle deleted the fix-download-eez-locally branch July 22, 2021 08:26
jnnr pushed a commit to jnnr/euro-calliope that referenced this pull request Aug 27, 2024
…-eez-locally

Fix download of EEZ to happen on local (login) node
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants