Added Drexel University Picotte template. #561
Conversation
erinvchen
requested review from
bdice and
SchoeniPhlippsn
and removed request for
a team
for more information, see https://pre-commit.ci
|
I worked on this with @erinvchen during office hours. I'm planning to make a few small changes to add this to the signac-flow docs. |
…c-flow into feature/picotte-template
|
@erinvchen Could you add yourself to the list of contributors? That's the last step needed for this PR. @glotzerlab/signac-committers This is ready for review. Since I helped write this, could I ask another committer to review this template? @erinvchen has verified that it works on the Picotte cluster for CPU and GPU submissions. |
There was a problem hiding this comment.
This needs a second review but I think it is ready to merge, aside from adding @erinvchen to the contributors list. The docs appear to render correctly.
The link to the cluster documentation is not publicly accessible - I think it may require Drexel network access. However, this is the canonical source for cluster information (@erinvchen screen-shared with me while we wrote the template). I think it's fine to leave the link as-is.
Codecov Report
@@ Coverage Diff @@
## master #561 +/- ##
==========================================
+ Coverage 77.51% 77.70% +0.18%
==========================================
Files 30 31 +1
Lines 3046 3054 +8
Branches 577 577
==========================================
+ Hits 2361 2373 +12
+ Misses 529 525 -4
Partials 156 156
Continue to review full report at Codecov.
|
@bdice Maybe add a quick note about this wherever the URL appears just so that users and future developers are not confused. |
bdice
changed the title
|
@erinvchen Can you add yourself to the contributors list? Then we can merge and release this PR. |
| {% set nn = nn|default((nn_cpu, nn_gpu)|max, true) %} | ||
| {% if partition == 'gpu' %} | ||
| #SBATCH --nodes={{ nn|default(1, true) }} | ||
| #SBATCH --ntasks-per-node={{ (gpu_tasks, cpu_tasks)|max }} |
There was a problem hiding this comment.
Does this correctly handle multi node GPU submissions?
There was a problem hiding this comment.
@b-butler Perhaps not — are you thinking that we are missing “divide tasks by the computed number of nodes and round up”? If you see something else, please clarify.
I believe this logic is copied from other templates so we should check carefully.
There was a problem hiding this comment.
Yes, we do this below for the dft partition. We do this in GreatLake's template, but I think we also error there. However, we don't deal with multi node submissions there since we don't have enough resource assess for that. An example of the other is the Expanse template.
There was a problem hiding this comment.
This will be handled separately in #566.
|
@erinvchen will you have time to implement these changes. |
|
@erinvchen We're going to merge this once tests pass - feel free to add yourself to the contributors list later, if you wish. |
Description
We added a template.
Motivation and Context
We're using this for simulations on the Picotte cluster.
Types of Changes
1The change breaks (or has the potential to break) existing functionality.
Checklist:
If necessary: