pfmc-assessments / indexwc

Estimate indices of abundance for west coast fish species
2 stars 2 forks source link

[BUG]: boundaries being incorrectly removed for boundaries that do not include a portion of all states #33

Closed chantelwetzel-noaa closed 1 month ago

chantelwetzel-noaa commented 7 months ago

Is there an existing issue for this?

Current Behavior

I was trying to run an index for only the area south of 40.10 N. latitude and kept getting an error from the filter_boundaries function. When I investigated the filter_boundaries function looked for boundaries in Washington and then Oregon which were not included in the data I was passing resulting in the default coastwide boundary being cleared out.

Expected Behavior

I added a check that kept the boundaries data frame from being cleared out if an area was not present here 7d195207509ff02f482e75abc3fa1ad21a82b245. I suspect this check can be done more elegantly but wanted to offer a basic suggestion to start from.

Steps To Reproduce

No response

Environment

- OS:
- Node:
- npm:

Anything else?

No response

kellijohnson-NOAA commented 7 months ago

Thanks @chantelwetzel-noaa for this and the code to fix it. I am going to wait a tiny bit to merge this in to think about if my original logic is the best way to do this or if there is a more simple way to go about this such as just looking to the list of boundaries that are passed and determine if they are valid rather than looking to each boundary. Also, sorry that I introduced this bug in the first place. If you have any other ideas beyond your current fix that would be ideal please feel free to let me know.

kellijohnson-NOAA commented 2 months ago

@chantelwetzel-noaa I am finally trying to work through this now and the suggestions that you made to the code have introduced a new error unfortunately.

filter_boundaries(y = c(48,48), boundaries_data)
Error in if (names(boundaries) %in% c("CA", "ca", "california", "California")) { : 
  the condition has length > 1

I think I have a way forward though.

chantelwetzel-noaa commented 2 months ago

One person's solution is another person's problem. Let me know if I can help in any way.

kellijohnson-NOAA commented 2 months ago

One thing that would be helpful is if you could provide a short vector of "y" values that was causing the problem in the original method.

chantelwetzel-noaa commented 2 months ago

I am unable to access my desktop to see how I was trying to run this code. I have been unable to recreate the error this morning. I will try to add an example next week.

kellijohnson-NOAA commented 2 months ago

Sounds good. Thank you.

chantelwetzel-noaa commented 2 months ago

I was able to find an example of when I was running into issues:

species_data <- nwfscSurvey::pull_catch(common_name = "yelloweye rockfish", survey = "NWFSC.Combo")
configuration <- configuration[which(configuration$species %in% "yelloweye rockfish"), ]
configuration$min_latitude <- 42.0 
data <-configuration %>%
  # Row by row ... do stuff then ungroup
  dplyr::rowwise() %>%
  # Pull the data based on the function found in fxn column
  dplyr::mutate(
    data_raw = list(format_data(data = species_data)),
    data_filtered = list(data_raw %>%
                           dplyr::filter(
                             depth <= min_depth, depth >= max_depth,
                             latitude >= min_latitude, latitude <= max_latitude,
                             year >= min_year, year <= max_year
                           ))
  ) %>%
  dplyr::ungroup()

best <- data %>%
  dplyr::mutate(
    # Evaluate the call in family
    family = purrr::map(family, .f = ~ eval(parse(text = .x))),
    # Run the model on each row in data
    results = purrr::pmap(
      .l = list(
        data = data_filtered,
        formula = formula,
        family = family,
        anisotropy = anisotropy,
        n_knots = knots,
        spatiotemporal = purrr::map2(spatiotemporal1, spatiotemporal2, list)
      ),
      .f = indexwc::run_sdmtmb
    )
  )
• Running sdmTMB for yelloweye rockfish
Error in `dplyr::mutate()`:
ℹ In argument: `results = purrr::pmap(...)`.
Caused by error in `purrr::pmap()`:
ℹ In index: 1.
Caused by error in `names(results) <- names(boundaries)`:
! 'names' attribute [1] must be the same length as the vector [0]
Run `rlang::last_trace()` to see where the error occurred.
chantelwetzel-noaa commented 1 month ago

@kellijohnson-NOAA I wanted to follow up on this to see if you need any additional information from me. Thanks.

kellijohnson-NOAA commented 1 month ago

Finally making progress, just running some local tests now but things look promising.

kellijohnson-NOAA commented 1 month ago

And, thank you @chantelwetzel-noaa for your example. It was super helpful because I had no idea we would ever get a boundary of upper 42 degrees and lower 42 degrees because of a single point right on the OR--CA border. I added a check to remove boundaries where the upper and lower limits are equal.