Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issues running vignette #13

Open
mkapur-noaa opened this issue Oct 11, 2023 · 5 comments
Open

issues running vignette #13

mkapur-noaa opened this issue Oct 11, 2023 · 5 comments

Comments

@mkapur-noaa
Copy link

mkapur-noaa commented Oct 11, 2023

Hi There
I'm working through the vignette and noticed a few issues. Some have been reported by other users, but I noticed that a couple are >2 years old and haven't yet been addressed. For that reason, I'm opening a single issue with several notes.

Would you recommend that users clone & modify their own local version of the package, if this version is no longer to be maintained? I understand it's a big lift to maintain a package, so wanted to check before moving forward for research purposes.

Here's what I've come upon. Will continue to add as I find issues.

  • Big-picture, the package depends on two deprecated CRAN pacakges: RandomFields and sf. The former must be manually installed from the tarball archive.
  • The vignette does not have arguments passed to init_popfor wt nor wtm1; the default values for these are of length 1 which is not the same length as the number of species in the example (2). The user needs to pass c('spp1' = 1, 'spp2' = 1) (or some other number) to both variables for the vignette to run.
  • As indicated by @Blevy2 in this issue, the number of fishing days is fixed to five. This causes the vignette to fail if the user changes the number of fishing days when init_fleet is run since there's a mismatch in the dimensions.
  • run_sim fails with an error about catches not being the right dimensions; it took me several hours to track this down to an issue online 5 of sum_fleet_catches where it wants to sum across sp_fleets_catches though those values are all NULL (they don't seem to get populated by the go_fish routine).
  • I recognize that an example of init_survey is not in the vignette, but when I run the function with the default settings, run_sim fails at line 421 because 1) the default output for Q is of length 1 (not 2, one for each species) and 2) the colnames under survey$surveysettings needs to match the lookup syntax ("Qs.spp1"), not "Qs1".
@pdolder
Copy link
Owner

pdolder commented Oct 12, 2023

Hi,

Thanks for the interest - you're right I haven't been actively maintaining the package, as I wasn't aware of others using it at present. I had fixed some of the issues that Ben flagged, so will check your list here and where the fixes have been implemented. In some cases it may be simply fixing the vignette to reflect the updates...

By coincidence I have addressed some of the indexing issues on a branch recently, but had some got round to folding into the main branch.

Apologies I haven't had much time today (in the middle of an assessment) but will have a look at this all tomorrow and get back to you with a detailed response.

@mkapur-noaa
Copy link
Author

Thanks so much for the reply - no worries. I made a fork last night (US West Coast time) but still wasn't able to sort whatever the issue is with run_sim that fails (in the vignette). In any case, I totally understand you have conflicting priorities (it's assessment time for us too) and do let me know if you're able to sort things out for the vignette. It's a great tool!

@pdolder
Copy link
Owner

pdolder commented Oct 12, 2023 via email

@pdolder
Copy link
Owner

pdolder commented Oct 13, 2023

Hi,

I've taken a look at this:

  • Big-picture, the package depends on two deprecated CRAN pacakges: RandomFields and sf. The former must be manually installed from the tarball archive.

I need to replace these packages, but am yet to identify a suitable replacement. They are only used one, and it's possible just to define a function to do this job so I'll take a look at it and report back when I've a solution.

  • The vignette does not have arguments passed to init_popfor wt nor wtm1; the default values for these are of length 1 which is not the same length as the number of species in the example (2). The user needs to pass c('spp1' = 1, 'spp2' = 1) (or some other number) to both variables for the vignette to run.

Thanks, these are updated now.

Yes, this is because of a design decision that I now regret to base every index at the tow level! It needs to be redefined in init_sim, and shouldn't be too hard to do - again, I'll report back once fixed.

  • run_sim fails with an error about catches not being the right dimensions; it took me several hours to track this down to an issue online 5 of sum_fleet_catches where it wants to sum across sp_fleets_catches though those values are all NULL (they don't seem to get populated by the go_fish routine).

This is only because of the issue above, right? I can't replicate this error with the basic settings in the vignette....so its based on changing settings ? Can you advise what triggers this.

  • I recognize that an example of init_survey is not in the vignette, but when I run the function with the default settings, run_sim fails at line 421 because 1) the default output for Q is of length 1 (not 2, one for each species) and 2) the colnames under survey$surveysettings needs to match the lookup syntax ("Qs.spp1"), not "Qs1".

Hmm, I can provide better defaults as I've used this in the paper. Will report when fixed.

@Blevy2
Copy link

Blevy2 commented Oct 13, 2023

Hello,

I had MixFishSim running smoothly and you are welcome to look at any of the files in my repository here: https://github.com/Blevy2/READ-PDB-blevy2-MFS2/tree/main/R

The above link contains all the MixFishSim files. Any that I edited have a "BENS" tacked onto the end. I did edit init_sim and several other init files to meet my needs.

As far as the RandomFields package, I am not sure what @mkapur-noaa is using MixFishSim for, but the way I used the package meant I did not need to use RandomFields. RandomFields is used to create Hab and/or Tol in the model, but I was reading in a Hab/Tol that I created on my own to represent the fish/region I was modeling. These are also in my repository if you wanted to use them or see what I did.

I really loved using the package and am happy to chat about how to use it or improve it. Sorry I did not suggest specific changes previously, I am just not very savvy with using github.

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

No branches or pull requests

3 participants