✨ Quality: Acknowledge that the Sieve of Eratosthenes implementation is "unfaithful"#1154
Conversation
…s "unfaithful" The current text in section 3.5.2 claims that the stream-based algorithm faithfully represents the Sieve of Eratosthenes. According to the referenced paper "The Genuine Sieve of Eratosthenes" (Melissa E. O'Neill, Journal of Functional Programming, 2009), this is incorrect. The algorithm presented is actually a trial division sieve, not the genuine Sieve of Eratosthenes. The text needs to be updated to reflect this, adding a note or modifying the existing description to acknowledge that while the algorithm is elegant and produces correct results, it is not a faithful representation of the original sieve algorithm. Affected files: subsection2.xml Signed-off-by: andoan16 <33853760+andoan16@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a footnote to clarify that the algorithm described is a trial division sieve rather than the genuine Sieve of Eratosthenes, citing research by Melissa E. O'Neill. The review feedback suggests moving the footnote to follow the term it clarifies for better readability and structural consistency.
| numbers, using a method known as the | ||
| <INDEX>prime number(s)<SUBINDEX>Eratosthenes<APOS/>s sieve for</SUBINDEX></INDEX> | ||
| <INDEX>sieve of Eratosthenes</INDEX> | ||
| <FOOTNOTE>This is not actually the Sieve of Eratosthenes, but rather a trial division sieve. The genuine Sieve of Eratosthenes marks off multiples of each prime starting from the prime's square, whereas this algorithm tests each candidate number by dividing it by all previously found primes. See Melissa E. O'Neill, <QUOTE>The Genuine Sieve of Eratosthenes,</QUOTE> <EM>Journal of Functional Programming</EM>, 2009.</FOOTNOTE> |
There was a problem hiding this comment.
The placement of this footnote is unconventional. For better readability, a footnote should typically be placed immediately after the term it clarifies, not before.
I recommend moving this <FOOTNOTE> to be after </EM> on line 236. The structure would then look like this:
...
<EM>sieve of
Eratosthenes</EM><FOOTNOTE>This is not actually the Sieve of Eratosthenes...</FOOTNOTE>.<FOOTNOTE>Eratosthenes, ...</FOOTNOTE>
...This will correctly associate the footnote with the term "sieve of Eratosthenes".
RichDom2185
left a comment
RichDom2185
left a comment
There was a problem hiding this comment.
Note: While we are open to entirely AI-generated PRs, please indicate as such clearly. Don't just make us infer it from the branch name.
| numbers, using a method known as the | ||
| <INDEX>prime number(s)<SUBINDEX>Eratosthenes<APOS/>s sieve for</SUBINDEX></INDEX> | ||
| <INDEX>sieve of Eratosthenes</INDEX> | ||
| <FOOTNOTE>This is not actually the Sieve of Eratosthenes, but rather a trial division sieve. The genuine Sieve of Eratosthenes marks off multiples of each prime starting from the prime's square, whereas this algorithm tests each candidate number by dividing it by all previously found primes. See Melissa E. O'Neill, <QUOTE>The Genuine Sieve of Eratosthenes,</QUOTE> <EM>Journal of Functional Programming</EM>, 2009.</FOOTNOTE> |
There was a problem hiding this comment.
📝 Addressed this feedback in commit fix: address review feedback — Please reformat it.. Thanks for the review!
Signed-off-by: andoan16 <33853760+andoan16@users.noreply.github.com>
Signed-off-by: andoan16 <33853760+andoan16@users.noreply.github.com>
2 participants
Problem
The current text in section 3.5.2 claims that the stream-based algorithm faithfully represents the Sieve of Eratosthenes. According to the referenced paper "The Genuine Sieve of Eratosthenes" (Melissa E. O'Neill, Journal of Functional Programming, 2009), this is incorrect. The algorithm presented is actually a trial division sieve, not the genuine Sieve of Eratosthenes. The text needs to be updated to reflect this, adding a note or modifying the existing description to acknowledge that while the algorithm is elegant and produces correct results, it is not a faithful representation of the original sieve algorithm.
Severity:
mediumFile:
xml/chapter3/section5/subsection2.xmlSolution
The current text in section 3.5.2 claims that the stream-based algorithm faithfully represents the Sieve of Eratosthenes. According to the referenced paper "The Genuine Sieve of Eratosthenes" (Melissa E. O'Neill, Journal of Functional Programming, 2009), this is incorrect. The algorithm presented is actually a trial division sieve, not the genuine Sieve of Eratosthenes. The text needs to be updated to reflect this, adding a note or modifying the existing description to acknowledge that while the algorithm is elegant and produces correct results, it is not a faithful representation of the original sieve algorithm.
Changes
xml/chapter3/section5/subsection2.xml(modified)Testing
Closes #1115