feat: admin talk #177
feat: admin talk #177
Conversation
…teaching-material into feat/admin_talk
|
Warning Rate limit exceeded@cmeesters has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes across multiple files, primarily focusing on enhancing course configuration, workflow documentation, and presentation materials. The changes include a new configuration file for an academic course, updates to slide presentations about Snakemake and workflow management, and improvements to LaTeX preamble and syntax highlighting. The modifications aim to provide more detailed, structured information about workflow management, HPC resources, and course presentations. Changes
Sequence DiagramsequenceDiagram
participant User
participant Workflow Manager
participant HPC Cluster
participant Software Environment
User->>Workflow Manager: Define workflow
Workflow Manager->>Software Environment: Request environment setup
Software Environment-->>Workflow Manager: Prepare Conda/EESSI environment
Workflow Manager->>HPC Cluster: Submit job
HPC Cluster-->>Workflow Manager: Execute workflow
Workflow Manager-->>User: Return results
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Artifacts |
…teaching-material into feat/admin_talk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
images/workflows/pure_SLURM.py (1)
23-23: Consider adjusting the annotation position for better visibility.The annotation "lines of code" is placed at (10,10) in figure pixels, which is very close to the figure edge. Consider increasing these coordinates for better visibility and readability.
-ax.annotate("lines of code", xy=(10,10), xycoords="figure pixels") +ax.annotate("lines of code", xy=(30,30), xycoords="figure pixels")config/config_Erlangen_Admin.yaml (4)
1-4: Fix trailing spaces in the instructor section.The section is well-structured, but there are trailing spaces that should be removed.
instructor: name: "Dr. Christian Meesters" affiliation: "Johannes Gutenberg Universität Mainz" - affiliation2: "NHR SüdWest" # may be empty + affiliation2: "NHR SüdWest" # may be empty
6-20: Standardize spacing in color definitions.The color definitions have inconsistent spacing after colons. Let's standardize them for better readability.
# choose any preconfigured color or those from the preamble - beamercolor_structure: "fg=UniRot" - beamercolor_title: "fg=UniRot" + beamercolor_structure: "fg=UniRot" + beamercolor_title: "fg=UniRot" beamercolor_title_head: "fg=UniRot" beamercolor_block_title: "bg=UniRot!20,fg=darkred" - beamercolor_block_body: "fg=black, bg=plightgrey2" + beamercolor_block_body: "fg=black, bg=plightgrey2"🧰 Tools
🪛 yamllint (1.35.1)
[warning] 12-12: too many spaces after colon
(colons)
[error] 12-12: trailing spaces
(trailing-spaces)
[warning] 13-13: too many spaces after colon
(colons)
[warning] 17-17: too many spaces after colon
(colons)
43-54: Consider making resource paths more configurable.The hardcoded absolute paths might make the configuration less portable across different HPC systems. Consider:
- Using environment variables or relative paths where possible
- Moving common path prefixes to a separate configuration
- Supporting both absolute and relative paths
Example approach:
course: base_path: ${NHR_WORKFLOW_PATH:-/lustre/project/nhr-workflow} softwarepath: "${base_path}/software_stack" pathtoworkflow: "${base_path}/workflow" pathtosetup: "${base_path}/tasks" pathtosolutions: "${base_path}/solutions"
24-27: Remove trailing spaces from commented lines.Clean up trailing spaces in the aspect ratio and title configurations.
- aspectratio: 1609 # - 169 = 16:9 + aspectratio: 1609 # - 169 = 16:9 - #title: "" # specifiy your title here, else a default is used. + #title: "" # specifiy your title here, else a default is used. - #subtitle: "" # specifiy your subtitle here, else a default is used. + #subtitle: "" # specifiy your subtitle here, else a default is used.🧰 Tools
🪛 yamllint (1.35.1)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
images/humor/Leonardo_Depict_former_French_Queen_Marie_Antoinett.jpgis excluded by!**/*.jpgimages/workflows/SLURM_workflow_solution.pngis excluded by!**/*.pngimages/workflows/proteo_transcriptomics.jpegis excluded by!**/*.jpegimages/workflows/proteotranstomic.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (9)
config/config_Erlangen_Admin.yaml(1 hunks)images/workflows/pure_SLURM.py(1 hunks)slides/Snakemake_HPC_Admins.tex(2 hunks)slides/admin/About_Snakemake.tex(3 hunks)slides/admin/Why_Workflows.tex(5 hunks)slides/admin/Workflow_Parameterization_for_HPC.tex(3 hunks)slides/admin/condarc_mogon.tex(1 hunks)slides/admin/software_environment.tex(2 hunks)slides/common/preamble.tex(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
images/workflows/pure_SLURM.py (1)
Pattern **/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 yamllint (1.35.1)
config/config_Erlangen_Admin.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
[warning] 12-12: too many spaces after colon
(colons)
[error] 12-12: trailing spaces
(trailing-spaces)
[warning] 13-13: too many spaces after colon
(colons)
[warning] 17-17: too many spaces after colon
(colons)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (14)
slides/admin/condarc_mogon.tex (1)
20-20: LGTM! Clear and accurate explanation.The updated explanation about environment storage location is more generic and clearer, helping users understand the purpose of the env_prompt setting.
slides/Snakemake_HPC_Admins.tex (2)
19-19: LGTM! Title update improves clarity.The updated title better reflects the target audience by including both admins and HPC users.
46-48: LGTM! New section adds relevant content.The addition of the actual_work section enhances the presentation structure.
slides/admin/software_environment.tex (2)
40-40: LGTM! Clear indication of new deployment method.The addition of "Conda Translation to EESSI" is properly marked as in development, providing users with information about upcoming features.
53-124: Consider preserving removed content in documentation.While removing the detailed Conda/Mamba initialization content helps streamline the presentation, this information might be valuable for users. Consider moving it to separate documentation or an appendix.
slides/admin/About_Snakemake.tex (1)
43-43: LGTM! Statistics updated and accessibility improved.The updates accurately reflect Snakemake's growth with:
- Increased citation count (>3000)
- Expanded workflow catalogue (>3000 workflows)
- Added hyperlink for better accessibility
Also applies to: 82-83, 92-92
slides/admin/Workflow_Parameterization_for_HPC.tex (2)
2-16: LGTM! Enhanced presentation structure and visual appeal.The changes effectively improve the slide organization:
- More focused section title for HPC users
- Relevant background image illustrating bottlenecks
- Well-formatted table of contents
125-139: LGTM! Valuable practical example added.The new frame provides essential practical guidance:
- Clear example of Snakemake command usage
- Well-explained parameters
- Important HPC-specific considerations
slides/common/preamble.tex (1)
158-194: LGTM! Comprehensive YAML syntax highlighting added.The new configuration provides:
- Well-defined color schemes for YAML elements
- Complete language definition for proper syntax highlighting
- Consistent styling rules
slides/admin/Why_Workflows.tex (3)
2-16: LGTM! Enhanced section focus and visual appeal.The changes effectively improve the presentation:
- More precise section title
- Engaging LEGO scientist background
- Well-structured table of contents
37-66: LGTM! Improved visual layout and organization.The changes enhance the presentation with:
- Better space utilization
- Clear image placement
- Proper source attribution and links
Line range hint
1074-1227: LGTM! Enhanced practical context and reproducibility focus.The changes effectively improve the content with:
- Clear batch system example
- Well-structured visual aids
- Strong emphasis on reproducibility principles
config/config_Erlangen_Admin.yaml (2)
1-74: Well-structured configuration with comprehensive documentation.The configuration file is well-organized and thoroughly documented. The separation into logical sections (instructor, layout, course, cluster) makes it easy to understand and maintain.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 5-5: trailing spaces
(trailing-spaces)
[warning] 12-12: too many spaces after colon
(colons)
[error] 12-12: trailing spaces
(trailing-spaces)
[warning] 13-13: too many spaces after colon
(colons)
[warning] 17-17: too many spaces after colon
(colons)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 26-26: trailing spaces
(trailing-spaces)
[error] 27-27: trailing spaces
(trailing-spaces)
71-72: Verify storage paths exist and are writable.The local and remote storage paths should be validated at runtime to ensure they:
- Exist and are accessible
- Have sufficient permissions
- Have adequate storage capacity
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
slides/admin/Why_Workflows.tex (1)
Line range hint
1074-1103: Consider adjusting vertical spacing for better readability.The frame effectively illustrates the complexity of batch system workflows. However, the vertical spacing between columns could be improved.
- \vspace{-12.5em} + \vspace{-11em}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
slides/admin/Why_Workflows.tex(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (4)
slides/admin/Why_Workflows.tex (4)
2-16: Well-structured section introduction!The new section title and layout with the DALLE-generated background image create an engaging opening. The table of contents is properly formatted and the image attribution is correctly included.
37-65: Well-implemented slide transitions and layout!The frame effectively uses tikzpicture for precise image positioning and onlyenv for slide transitions. The image attributions are properly handled with minipage environments.
Line range hint
1104-1156: Excellent structure for discussion-oriented slides!The frames effectively use pauses and itemize environments to build a compelling narrative about HPC user expectations and planning. The questions are well-crafted to stimulate discussion.
1157-1228: Excellent presentation of reproducibility concepts!The frames effectively use visual elements and progressive disclosure to explain reproducibility concepts. The proper attribution and linking to the Snakemake paper adds credibility to the content.
…teaching-material into feat/admin_talk
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…teaching-material into feat/admin_talk
|
@coderabbitai I think, this PR is ready to be merged. However, it appears that the slides do not get rendered for a release PR merger. Any ideas how to solve this issue? |
|
@cmeesters I've found the issue. There are actually two problems in the workflow configurations:
To fix this, you'll need to:
Would you like me to create an issue to track these workflow fixes? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 6478 ✅ Actions performedComments resolved. |
|
@coderabbitai - thank you. I followed your suggestions. Please review. |
|
Tip For best results, initiate chat on the files or code changes. @cmeesters Thank you for making the changes. I see that the
Would you like me to create an issue to track these remaining workflow fixes? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 4784 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/release-please.yml (1)
Line range hint
1-73: Improve workflow structure and job coordination.The workflow needs better coordination between the build and release jobs:
- The release information isn't properly passed between jobs
- The condition in the build job can't access the release job's outputs
Consider restructuring the workflow to:
- Move the artifact upload step to the release job
- Use job outputs to coordinate between jobs
Here's a suggested workflow structure:
jobs: build: name: Release Artifacts runs-on: ubuntu-latest outputs: pdfs_created: ${{ steps.build.outputs.pdfs_created }} steps: - uses: actions/checkout@v4 # ... existing build steps ... - name: Build PDFs id: build run: | # ... existing build commands ... echo "pdfs_created=true" >> $GITHUB_OUTPUT release-please: runs-on: ubuntu-latest needs: build outputs: release_created: ${{ steps.release.outputs.release_created }} steps: - uses: GoogleCloudPlatform/release-please-action@v4 id: release # ... existing release config ... - if: ${{ steps.release.outputs.release_created }} name: Download Artifacts uses: actions/download-artifact@v4 with: name: Slides - if: ${{ steps.release.outputs.release_created }} name: Create Release Asset run: zip artifacts slides/*pdf - if: ${{ steps.release.outputs.release_created }} name: Upload Release Asset uses: actions/upload-release-asset@v1 # ... existing upload config ...🧰 Tools
🪛 actionlint (1.7.4)
50-50: could not parse as YAML: yaml: line 50: mapping values are not allowed in this context
(syntax-check)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
images/humor/announcement.pngis excluded by!**/*.png
📒 Files selected for processing (2)
.github/workflows/build.yml(0 hunks).github/workflows/release-please.yml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/build.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release-please.yml
50-50: could not parse as YAML: yaml: line 50: mapping values are not allowed in this context
(syntax-check)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai your suggestion do not appear to work, see https://github.com/snakemake/snakemake-hpc-teaching-material/actions/runs/12967077300/job/36168584395 |
|
Tip For best results, initiate chat on the files or code changes. @cmeesters I found several issues in the workflow file that are causing the failure:
Here's the corrected workflow file: on:
push:
branches:
- main
name: release-please
permissions:
contents: write
pull-requests: write
jobs:
release-please:
runs-on: ubuntu-latest
outputs:
release_created: ${{ steps.release.outputs.release_created }}
upload_url: ${{ steps.release.outputs.upload_url }}
steps:
- uses: GoogleCloudPlatform/release-please-action@v4
id: release
with:
release-type: simple
build:
needs: release-please
if: ${{ needs.release-please.outputs.release_created }}
name: Release Artifacts
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install Dependencies
run: sudo apt-get update && sudo apt-get install -y --no-install-recommends
python3-yaml
texlive
texlive-fonts-extra
texlive-latex-extra
texlive-luatex
texlive-pictures
texlive-plain-generic
texlive-pstricks
texlive-science
latexmk
ghostscript
poppler-utils
- name: LaTeX compile
run: |
./render --master-tex Snakemake_HPC_Users.tex \
--configfile config/config_for_github.yaml ;
./render --master-tex Snakemake_HPC_Creators.tex \
--configfile config/config_for_github.yaml;
./render --master-tex Snakemake_HPC_User_Creator_Combi.tex \
--configfile config/config_for_github.yaml
./render --master-tex Snakemake_HPC_Admins.tex \
--configfile config/config_for_github.yaml;
zip slides.zip slides/*pdf
- name: Upload Release Artifacts
uses: actions/upload-artifact@v4
with:
name: Slides
path: |
slides/Snakemake_HPC_Creators.pdf
slides/Snakemake_HPC_Users.pdf
slides/Snakemake_HPC_User_Creator_Combi.pdf
slides/Snakemake_HPC_Admins.pdf
- uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ needs.release-please.outputs.upload_url }}
asset_path: ./slides.zip
asset_name: artifacts.zip
asset_content_type: application/zipKey changes:
Would you like me to create an issue to track these workflow changes? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 16688 Script: Length of output: 2561 ⛔ Permission issueThere was a permission issue. Please ask the repository administrator to refresh CodeRabbit's permissions at https://github.com/apps/coderabbitai/installations/53720561. |
the talk to admins needs an update - the PR is still work in progress
Summary by CodeRabbit
New Features
Documentation
Chores