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

Issue with definition of Barrier_scaling used in applications of Barrier mesh option #330

Open
N-DucharmeBarth-NOAA opened this issue Jun 28, 2022 · 3 comments

Comments

@N-DucharmeBarth-NOAA
Copy link

I had a look in the code and see where Barrier_scaling is defined in make_data() on line 1014 as:

Return[['Barrier_scaling']] = c(0.2, 1)

and where it enters the VAST code as c on lines 28-30 of VAST_v13_1_0.cpp:

  vector <Type> range(2);
  range(0) = sqrt(8.0)/kappa*c(0);
  range(1) = range(0)*c(1);

Barrier_scaling should be c(1,0.2) following the original Brevik paper (Equation 3) and line 62 from this Brevik & Skaug example.

Additionally, the fraction of the spatial range of the decorrelation distance applied in the presence of a barrier could be user specified (e.g. Return[['Barrier_scaling']] = c(1, range_fraction)). range_fraction could be defined as a formal argument to make_data() with a default value of 0.2.

@James-Thorson-NOAA
Copy link
Owner

Nicholas,

Thanks for pointing this out! Using the Pacific Islands grid and barrier vs. non-barrier fits that you provided, I can confirm that the previous implementation was a bug, and that I need to change the Barrier_scaling vector as you say.

I'll aim to do a hotfix soon (likely next week), and will leave this open until then.

@James-Thorson-NOAA
Copy link
Owner

@N-DucharmeBarth-NOAA

Do you mind looking at my lastest push for the main branch of VAST to see if it fits your expectations? I added Range_fraction as an option to pass via settings$Options (with reference documentation updated) but changed the default as noted.

If it looks good (or you don't have time), please respond and I'll make a numeric release

@N-DucharmeBarth-NOAA
Copy link
Author

Hi Jim,
I took a look at the latest code change and it looks good to me. I'll test it on my next model runs. Thank you for making that update!
Best,
Nicholas

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

2 participants