Skip to content

Random sampling of NSHM rupture magnitude - #133

Open
lispandfound wants to merge 9 commits into
pegasusfrom
magnitude_sampling_nshm
Open

Random sampling of NSHM rupture magnitude#133
lispandfound wants to merge 9 commits into
pegasusfrom
magnitude_sampling_nshm

Conversation

@lispandfound

Copy link
Copy Markdown
Contributor

Generates a random draw of the magnitude when --magnitude-strategy is set to random.

Copilot AI lite review requested due to automatic review settings August 27, 2026 22:34
@lispandfound

Copy link
Copy Markdown
Contributor Author

@claude review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds support for sampling a random NSHM rupture magnitude when --magnitude-strategy is set to random, complementing the existing rupture propagation sampling strategy.

Changes:

  • Introduces a magnitude_strategy CLI option intended to control how total rupture magnitude is chosen (median vs random draw).
  • Extends default_magnitude_estimation to request a random magnitude draw via magnitude_scaling.area_to_magnitude(..., random=...).
  • Seeds Python’s random RNG alongside NumPy to support reproducible random sampling.
Suppressed comments (2)

workflow/scripts/nshm2022_to_realisation.py:82

  • default_magnitude_estimation adds a required strategy parameter, but the docstring’s Parameters section doesn’t describe it, which makes it hard to understand what "maximising" vs "random" means for magnitude sampling.
    components: "DisjointSet[str]",
    avg_rake: float,
    strategy: SamplingStrategy,
) -> dict[str, magnitude_scaling.BoldM]:

workflow/scripts/nshm2022_to_realisation.py:262

  • The magnitude_strategy docstring says that "random" will choose a random rupture propagation tree, but this option is for magnitude sampling. This is likely a copy/paste error and could confuse CLI users.
    magnitude_strategy : SamplingStrategy, optional
        The strategy to use when sampling total rupture magnitude. "maximising"
        will choose the median rupture magnitude. "random" will choose a random
        rupture propagation tree.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 321 to 325
faults_info = db.get_rupture_fault_info(rupture_id)
seeds = Seeds.read_from_realisation_or_random(realisation_ffp)
np.random.seed(seed=seeds.nshm_to_realisation_seed)
random.seed(seeds.nshm_to_realisation_seed)
source_config = SourceConfig(faults)
Comment thread workflow/scripts/nshm2022_to_realisation.py
@AndrewRidden-Harper
AndrewRidden-Harper self-requested a review August 28, 2026 02:30

@AndrewRidden-Harper AndrewRidden-Harper left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot has some comments, so just check those if you haven't already. Otherwise, it looks good

lispandfound and others added 4 commits August 28, 2026 16:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

workflow/scripts/nshm2022_to_realisation.py:262

  • The magnitude_strategy parameter docstring incorrectly says the "random" option chooses a random rupture propagation tree; it should describe random sampling of rupture magnitude.
        The strategy to use when sampling rupture propagation. "maximising" will
        choose the maximally likely rupture propagation tree. "random" will
        choose a random rupture propagation tree.

Comment thread workflow/scripts/nshm2022_to_realisation.py
@AndrewRidden-Harper
AndrewRidden-Harper self-requested a review August 28, 2026 04:36
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.

3 participants