Skip to content

Address comments on episode 4 - #17

Open
stderr-enst wants to merge 5 commits into
mainfrom
review-04-answer
Open

Address comments on episode 4#17
stderr-enst wants to merge 5 commits into
mainfrom
review-04-answer

Conversation

@stderr-enst

Copy link
Copy Markdown
Collaborator

Addressing #8

@stderr-enst
stderr-enst requested a review from GayatriManda July 13, 2026 09:42
@github-actions

Copy link
Copy Markdown

🆗 Pre-flight checks passed 😃

This pull request has been checked and contains no modified workflow files or spoofing.

Results of any additional workflows will appear here when they are done.

@stderr-enst stderr-enst mentioned this pull request Jul 13, 2026
@stderr-enst stderr-enst changed the title Address Gayatris comments on episode 4 WIP: Address comments on episode 4 Jul 15, 2026
@stderr-enst

Copy link
Copy Markdown
Collaborator Author

I'll also add suggested changes from Julians review here

@GayatriManda GayatriManda left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So far, it looks good to me. Thank you for addressing my comments and making the changes.

Comment thread episodes/04_scalingstudy.md Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This instructor note could be moved to the Prerequisites or Introduction section, as it fits better there.

They have already reached the middle of the guide by this point, so it doesn't make much sense to introduce it here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good point, thanks!
I'll address this in a new commit to this PR.


So far, we kept the workload size fixed to $800 \times 800$ pixels and $128$ samples per pixel for the same scene with three snowman.
The diminishing returns for adding more and more parallel processors leads to a famous observation.
The speedup of a program through parallelization is limited by the execution time of the serial fraction that is not parallelizable.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So far, execution time has been used as the performance metric. Suddenly, speedup is introduced without any explanation. It would be helpful to first explain what speedup means, what the baseline is, and how it is calculated before using it in the discussion.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think you may have missed the speedup introduction at the beginning of the same section? At least the lines you are referring to here are after the explanation about speedup.

github-actions Bot pushed a commit that referenced this pull request Jul 21, 2026
@stderr-enst stderr-enst changed the title WIP: Address comments on episode 4 Address comments on episode 4 Jul 21, 2026
@stderr-enst

Copy link
Copy Markdown
Collaborator Author

Now this includes a lot of changes from @GayatriManda and @jjegg01 comments. Thanks again for the input. Could you have a look at it and let me know if you think this should be merged as is?

There are two bigger open points, addressed in Julians review issue:

  1. Rephrase both sections on strong and weak scaling in terms of $T(P, W)$ with $P$ parallel processors and workload $W$' I like the idea, but I'd avoid calling both $N$. And it'd need some time to make the naming consistent. (in plots, scripts, sections, etc.). This could also motivate some reordering in terms of better pacing. When to introduce a definition like strong or weak scaling, when and if to mention a certain mathematical description, where to discuss an example.
  2. Provide a streamlined analysis experience by providing some infrastructure and structure the whole episode around it. Maybe as an external git repo we pull, formally explain how to prepare the environment, or introduce it as a prerequisite in the setup section of the whole course. This way everyone could run the same script, with a clear and improved UX. We could also go into your very valid point of teaching how to prepare the benchmark/measurement code. This is a good example of how we sometimes need to develop glue code in HPC environments to make sense of things. We could even point at benchmarking tools like JUBE (i think Marc-Andrè was working on a carpentries course about this. I wouldn't branch off too far though and just mention it as a connection.)

Let me know, what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants