Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 455 426 -29
=========================================
- Hits 455 426 -29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@manodeep Something for you to review during the NCI maintenance ;) |
manodeep
left a comment
manodeep
left a comment
There was a problem hiding this comment.
Thanks @micaeljtoliveira - the simplification looks good to me! I have made one suggestion to see whether we can simplify even further by combining num_nodes and cores_per_node
micaeljtoliveira
force-pushed
the
esm_1.6_update
branch
from
119380a to
214f872
Compare
|
Marking as draft, as #37 needs to be merged first. |
micaeljtoliveira
force-pushed
the
esm_1.6_update
branch
from
214f872 to
489a81c
Compare
|
@manodeep I think all comments have been addressed and I took the opportunity to clean-up a bit the git history. Would you mind having a final look and approve? |
manodeep
left a comment
manodeep
left a comment
There was a problem hiding this comment.
@micaeljtoliveira Suggested one update to the docstrings - everything else looks good. Please feel free to add that typo-fix and merge after.
…tring. Renamed the start_blocknum parameter to start_seqnum.
…me. The number of cores per node is now passed as function paramenter instead of infered from the queue name.
…es the layouts for only one node count at a time, instead of doing it for a list of node counts, reducing code complexity.
Co-authored-by: Manodeep Sinha <manodeep@gmail.com>
micaeljtoliveira
force-pushed
the
esm_1.6_update
branch
from
489a81c to
4664902
Compare
|
@manodeep Thanks for the review! |
2 participants
Simplify a few things and make code a bit more general so that it's easier to integrate with the
ProfilingManagerfrom access.profiling.Depends on #37