pfmc-assessments / PacFIN.Utilities

R code to manipulate data from the PacFIN database for assessments
http://pfmc-assessments.github.io/PacFIN.Utilities
Other
7 stars 1 forks source link

`getComps()` uses `Final_Sample_Size_L` not `Final_Sample_Size`? #113

Closed iantaylor-NOAA closed 1 year ago

iantaylor-NOAA commented 1 year ago

The documentation of getComps() says that it uses Final_Sample_Size but the code seems to only use Final_Sample_Size_L.

The getComps() function is providing good expansions because the value of Final_Sample_Size_L is set to the product of expansion factors 1 and 2 in getExpansion_2(): https://github.com/pfmc-assessments/PacFIN.Utilities/blob/497185807b910cbe6664d3171508e432fe473330/R/getExpansion_2.R#L195-L196

To Reproduce Steps to reproduce the behavior:

  1. Run the attached script (relies on access to NWFSC server for data file): test_pacfin_expansion.R.txt
  2. Note that changing Final_Sample_Size didn't impact the age or length comps, but changing Final_Sample_Size_L impacted both age and length comps. Final_Sample_Size_A doesn't seem to be used anywhere in the package.

Expected behavior Either changing Final_Sample_Size should impact the comps or the documentation should be changed to clarify what sample size column to change. The only issue is if you want to provide a different calculation for Final_Sample_Size that the default, such as by changing the quantile passed to capValues() or using some other formula for the sample sizes.

kellijohnson-NOAA commented 1 year ago

Thank you @iantaylor-NOAA for reporting this lack of clarity. I think that there are multiple things going on here

iantaylor-NOAA commented 1 year ago

Thank you for taking this on @kellijohnson-NOAA. Looking around at the repositories for assessments being conducted now, I don't see folks changing the default Final_Sample_Size by very much so I think any changes will have only a minor impact. For petrale, I now understand how to create unexpanded comps for use in a sensitivity analysis using the status-quo package, but will happily revise the script as needed to work with any changes you make to this package to ensure things are reproducible in the future.

kellijohnson-NOAA commented 1 year ago

Thanks again @iantaylor-NOAA for reporting this. It appears to be strictly a documentation problem though I will admit I haven't worked all of the way through it yet. I hope to have a pull request to the package complete by tomorrow morning that updates the documentation. Users should be passing the column name that they want to use in getComps_long(weightid = ) to getComps(weightid = ""). The default is weightid = "Final_Sample_Size_L", which is why nothing changed for the age information. Pdata should have a column called Final_Sample_Size_A that should be used for age composition information.

iantaylor-NOAA commented 1 year ago

Good to hear that it won't be TOO much work to sort this out. Thank you @kellijohnson-NOAA.

brianlangseth-NOAA commented 1 year ago

Im a little confused. Ian suggested that this is only an issue when doing something other than the defaults, but it also seems like it is an issue when doing age comps. Can you confirm whether age comps need to be recalculated after Kelli makes the fix (of course the effect is probably not an overly large one - so whether it "needs" to be done is relative).

kellijohnson-NOAA commented 1 year ago

This is an ⏰ for any population with age data. Unless you set weightid = "" to something different than the default of weightid = "Final_Sample_Size_L", then your input sample sizes will be based on the weight of all lengthed fish rather than the weight of aged fish. Or, you can set it to any column you want, the benefit of using Final_Sample_Size_A is that you do not have to do the multiplication yourself.

You can go ahead and fix your results now with the current package structure. I am just updating the documentation is all.

Please check the box in front of your 🧑‍🤝‍🧑 🐟 acknowledging that you are aware of the issue and will do what you need to do so that I can 😴 at night.