Skip to content

Adds gamma distribution#31

Open
kellijohnson-NOAA wants to merge 1 commit into
mainfrom
feat-gamma
Open

Adds gamma distribution#31
kellijohnson-NOAA wants to merge 1 commit into
mainfrom
feat-gamma

Conversation

@kellijohnson-NOAA

Copy link
Copy Markdown
Contributor

What is the feature?

  • Adds the gamma distribution for sampling

How have you implemented the solution?

  • Using codex 😁

Does the PR impact any other area of the project, maybe another repo?

Comment thread R/sample_gamma.R

if (any(sd < 0)) {
cli::cli_abort(c(
"x" = "All values of {.var sd} must be positive",

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.

change to "All values of {.var sd} must be non-negative".

Comment thread R/sample_gamma.R
))
}

if (length(x) == 1 && length(sd) > 1) {

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.

If sd has length 1, it's not recycled to the length of x. If a user calls sample_gamma(1:5, sd = 1), it returns a vector of identical values: 0.8361776 0.8361776 0.8361776 0.8361776 0.8361776. A possible fix from Gemini:

if (length(x) != length(sd)) {
    if (length(x) == 1) {
      x <- rep(x, length(sd))
    } else if (length(sd) == 1) {
      sd <- rep(sd, length(x))
    } else {
      cli::cli_abort(c(
        "x" = "Length of {.var x} and {.var sd} must be compatible",
        "i" = "{.var x} has length {length(x)}",
        "i" = "{.var sd} has length {length(sd)}"
      ))
    }
  }

@Bai-Li-NOAA Bai-Li-NOAA 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.

Thanks, @kellijohnson-NOAA, for working on this issue. I have a few minor comments and requests regarding the recycling of sd when x is longer and some of the error messages.

#' @description Test that sample_gamma() returns a vector when a vector is
#' passed to `x` but a single value is passed to `sd`.
expect_equal(
object = length(sample_gamma(1:5, 1)),

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.

Add a test to check values from sample_gamma(1:5, 1).

@Bai-Li-NOAA Bai-Li-NOAA moved this from Backlog to In review in Ecosystem coding for assessments Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Add Gamma distribution sampling functionality

2 participants