Skip to content

Conversation

@nh13
Copy link
Owner

@nh13 nh13 commented Dec 22, 2025

Summary

  • Replaces srand48() with seed48() to initialize all 48 bits of the RNG state
  • Uses standard 0x330e value for upper 16 bits as per POSIX specification
  • Ensures consistent initialization across platforms

Fixes #106

Test plan

  • Unit tests pass (make test-unit)
  • Integration tests pass (make test-integration)
  • Verified reproducibility with explicit seed values

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • Improved random seed initialization to use a full 48-bit state for more robust, consistent, and reproducible random number generation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Warning

Rate limit exceeded

@nh13 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 4 minutes and 45 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 72961e7 and df1c3d9.

📒 Files selected for processing (1)
  • src/dwgsim_opt.c
📝 Walkthrough

Walkthrough

Replaces srand48() with seed48() in src/dwgsim_opt.c, building a 3-word xseed array (from provided seed or time, with upper word 0x330e) and calling seed48() to initialize the full 48-bit RNG state.

Changes

Cohort / File(s) Summary
RNG State Initialization
src/dwgsim_opt.c
Replaced srand48() with seed48() usage: constructs unsigned short xseed[3] from chosen seed (time-based when seed == -1 or provided), sets xseed[2] = 0x330e, and calls seed48(xseed) to fully initialize 48-bit RNG state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudged a seed into the loam at night,
Three little words now set the bits just right.
Forty-eight whispers spun from time and hand,
A hop, a hum — reproducible land. 🌿✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: replacing srand48() with seed48() for complete RNG state initialization, which directly aligns with the primary objective of the changeset.
Linked Issues check ✅ Passed The code changes in src/dwgsim_opt.c implement the proposed fix from issue #106 exactly: constructing an unsigned short xseed[3] array with seed high/low 16-bit words and 0x330e for the upper word, then calling seed48(xseed). Tests pass as required.
Out of Scope Changes check ✅ Passed All changes are directly related to issue #106's objective of replacing srand48() with seed48() for full 48-bit RNG state initialization; no out-of-scope modifications are present.

Comment @coderabbitai help to get the list of available commands and usage tips.

@nh13 nh13 force-pushed the fix/seed-initialization branch from 8266c32 to 34e4ecb Compare December 22, 2025 23:34
@nh13
Copy link
Owner Author

nh13 commented Dec 25, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 25, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
Copy link
Owner Author

nh13 commented Dec 26, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
Copy link
Owner Author

nh13 commented Dec 26, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26e3aa9 and 34e4ecb.

⛔ Files ignored due to path filters (5)
  • testdata/ex1.test.bfast.fastq.gz is excluded by !**/*.gz
  • testdata/ex1.test.bwa.read1.fastq.gz is excluded by !**/*.gz
  • testdata/ex1.test.bwa.read2.fastq.gz is excluded by !**/*.gz
  • testdata/ex1.test.mutations.txt.gz is excluded by !**/*.gz
  • testdata/ex1.test.mutations.vcf.gz is excluded by !**/*.gz
📒 Files selected for processing (1)
  • src/dwgsim_opt.c

@nh13 nh13 force-pushed the fix/seed-initialization branch from 34e4ecb to 72961e7 Compare December 26, 2025 21:44
srand48() only sets 32 bits of the 48-bit internal state, leaving the
upper 16 bits with potentially inconsistent values across platforms.
This fix uses seed48() with explicit state matching POSIX srand48()
behavior: X_i = (seedval << 16) + 0x330E

This ensures consistent, reproducible random sequences across all
platforms without changing the output for any given seed value.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@nh13 nh13 force-pushed the fix/seed-initialization branch from 72961e7 to df1c3d9 Compare December 26, 2025 21:54
@nh13 nh13 merged commit e772397 into main Dec 26, 2025
2 checks passed
@nh13 nh13 deleted the fix/seed-initialization branch December 26, 2025 21:54
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.

fix: use seed48() for complete RNG state initialization

2 participants