timjmiller / wham

State-space, age-structured fish stock assessment model
https://timjmiller.github.io/wham
Other
32 stars 16 forks source link

Issue when simulating paa and age-specific selectivity == 0 #80

Closed GiancarloMCorrea closed 1 year ago

GiancarloMCorrea commented 1 year ago

Let's assume we have age-specific selectivity for a given fleet, and selectivity is = 0 for ages 1 and 2. The set_osa_obs.R script will remove the paa observations for those ages in the 'obs' data frame.

Now, if we want to simulate paa data for that fleet, an issue arises here (same for surveys). The length of the vector 't_pred_paa' is = n_ages, but the length of the vector 'ages_obs_y' is = n_ages - 2 (ages 1 and 2 were removed). Therefore, these lines will be problematic: n_ages simulated values will be produced and sorted in a vector whose length = n_ages - 2.

I fixed this issue here, but want to confirm if this would be the best way to do it.

timjmiller commented 1 year ago

Thanks Giancarlo. Did you get this error using multionial, Dirichlet-multinomial, or MVTweedie? It looks like it might only be an issue with these likelihoods. The other likelihoods have an "ages" argument that can reduce the size of the returned observation vector.

On Mon, May 15, 2023 at 12:16 PM Giancarlo M. Correa < @.***> wrote:

Let's assume we have age-specific selectivity for a given fleet, and selectivity is = 0 for ages 1 and 2. The set_osa_obs.R script will remove the paa observations for those ages in the 'obs' data frame.

Now, if we want to simulate paa data for that fleet, an issue arises here https://github.com/timjmiller/wham/blob/de2e89fd329e4dc1c5e83c8ab99294ce01ee0ef6/src/wham_v0.cpp#LL1044C20-L1044C20 (same for surveys). The length of the vector 't_pred_paa' is = n_ages, but the length of the vector 'ages_obs_y' is = n_ages - 2 (ages 1 and 2 were removed). Therefore, these lines https://github.com/timjmiller/wham/blob/de2e89fd329e4dc1c5e83c8ab99294ce01ee0ef6/src/age_comp_sim.hpp#L313C11-L315 will be problematic: n_ages simulated values will be produced and sorted in a vector whose length = n_ages - 2.

I fixed this issue here https://github.com/GiancarloMCorrea/wham/blob/74f4a007d8656ec0fe74fd85598ad3f3c1a7808a/src/age_comp_sim.hpp#L330-L395, but want to confirm if this would be the best way to do it.

— Reply to this email directly, view it on GitHub https://github.com/timjmiller/wham/issues/80, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIGN7HP6QOUM44YFGZ5UY3XGJJFJANCNFSM6AAAAAAYCNRHLY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Timothy J. Miller, PhD (he, him, his) Research Fishery Biologist NOAA, Northeast Fisheries Science Center Woods Hole, MA 508-495-2365

GiancarloMCorrea commented 1 year ago

Hi Tim,

I used multinom. I agree, that issue will happen only with those options.

timjmiller commented 1 year ago

I have verified that it is also an issue for evaluating the likelihood when fitting this configuration. Testing a fix now.

On Mon, May 22, 2023 at 11:46 AM Giancarlo M. Correa < @.***> wrote:

Hi Tim,

I used multinom. I agree, that issue will happen only with those options.

— Reply to this email directly, view it on GitHub https://github.com/timjmiller/wham/issues/80#issuecomment-1557463328, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIGN7HVFZW33AYNNITZAA3XHOC6XANCNFSM6AAAAAAYCNRHLY . You are receiving this because you commented.Message ID: @.***>

-- Timothy J. Miller, PhD (he, him, his) Research Fishery Biologist NOAA, Northeast Fisheries Science Center Woods Hole, MA 508-495-2365

Cole-Monnahan-NOAA commented 1 year ago

When I do this the data are always. I've only experimented with this setup using the multinomial but it seemed to estimate fine.

timjmiller commented 1 year ago

This is now corrected on devel branch.