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

detection_range_model function #162

Merged
merged 124 commits into from
Sep 20, 2024
Merged

detection_range_model function #162

merged 124 commits into from
Sep 20, 2024

Conversation

jdpye
Copy link
Member

@jdpye jdpye commented Nov 30, 2021

In GitLab by @benjaminhlina on Nov 26, 2021, 11:30

detection_range_model uses a third-order polynomial, logit, or, probit model to determine detection range based on a expected percentage of detections for acoustic telemetry receivers. This function is to be used after a preliminary range test which uses multiple distances away from a representative receiver. Preliminary detection efficiency is to be determined in Vemco's Range Testing software and exported as a csv. Vignette on the use of the function is being written.

@jdpye jdpye changed the base branch from main to dev January 5, 2024 13:17
@jdpye
Copy link
Member Author

jdpye commented Jan 11, 2024

@benjaminhlina is looking to revamp this a bit before he officially submits it to glatos.

@jdpye jdpye marked this pull request as draft January 11, 2024 18:12
@jdpye jdpye self-assigned this Jan 11, 2024
Copy link
Collaborator

@chrisholbrook chrisholbrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several errors in the vignette and warnings were encountered that should be fixed (errors) or explained to the reader (OK for warnings if they are benign). There are also a number of R CMD CHECK notes, which I am am addressing and will push fixes for today.

map(~ st_buffer(dist = .x$distance, rcv_osc_sf_12)) %>%
bind_rows(.id = "distance") %>%
st_cast("LINESTRING") %>%
dplyr::select(distance, glatos_array, station_no, ins_serial_no, geometry)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning message:
In st_cast.sf(., "LINESTRING") :
repeating attributes for all sub-geometries for which they may not be constant

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common warning with st_cast() when geometry lengths differ.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is lower priority than other warnings I've flagged here (so OK releasing in this form with intent to clean up later), but readers not familiar with sf will appreciate you explaining to them what this means. In this particular case, this warning tells you that you are making an assumption that all geometries have the same attributes--there are consequences if this is not true. If we intend to show/teach best practices here, this should be cleaned up in the code (set attributes constant among geometries; or run the operation separately on each geometry) so that the warning isn't generated.

#| title: transform, view and filter

buffer_rings_pts <- buffer_rings %>%
st_cast("POINT") %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning message:
In st_cast.sf(., "POINT") :
repeating attributes for all sub-geometries for which they may not be constant

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common warning with st_cast() when geometry lengths differ.

#| title: save excel and gpx file

# save as excel
openxlsx::write.xlsx(deploy_sites, "YOUR_FILE_PATH.xlsx")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error in is.nan(tmp) : default method not implemented for type 'list'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if this chunk is meant to be as an example of what someone could use, the chunk has echo = TRUE and eval = FALSE so that when it knits the rmd it doesn't actual run but when running each chunk I found it also to produce the error. I can remove this all together if you'd like but originally wanted to demonstrate how to export these objects.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your reader doesn't know that eval = FALSE; that just prevents it from running/breaking on build. All code chunks should run, in order encountered in the document, without error. If not, it should be made very clear to the reader why not and give them some pointers on what to do about it. If that can't be done here, then I suggest just remove the code block and joint point the reader to openxlsx::write.xlsx() (in text, not code) as a function they might be able to use.

openxlsx::write.xlsx(deploy_sites, "YOUR_FILE_PATH.xlsx")

# save as gpx
st_write(deploy_sites, "YOUR_FILE_PATH", driver = "GPX")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: Layer creation failed.
In addition: Warning message:
In CPL_write_ogr(obj, dsn, layer, driver, as.character(dataset_options), :
GDAL Error 6: Field of name 'id' is not supported in GPX schema. Use GPX_USE_EXTENSIONS creation option to allow use of the element.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same goes for this chunk as well.

#| title: plot models
#| fig-height: 5
#| fig-width: 7
ggplot() +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning message:
In predictor + offset :
longer object length is not a multiple of shorter object length

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is causing this warning message as I cannot produce it on my end. I'll dig into it some more


```{r, warning = FALSE}
#| title: create redployment ring
redeploy_loc <- st_buffer(dist = 370, rcv_osc_sf_12) %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: object 'rcv_osc_sf_12' not found

If you start a new R script in Section 3 (see first text block in that section) then this object doesn't exist. This object either needs to be attached to the session or re-run the script in Section 2.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have amended the vignette to reload and create this object


# Analysis

For the analysis you will want to start a new R script, hence why I have left load packages below.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you truly start a new R script, then later in this section code fails with error:

Error: object 'rcv_osc_sf_12' not found

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have amended the vignette to reload and create this object so that it does not error considering the object cannot be found considering it isn't brought in.

```{r, warning = FALSE}
#| title: create redployment ring
redeploy_loc <- st_buffer(dist = 370, rcv_osc_sf_12) %>%
st_cast("LINESTRING") %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning message:
In st_cast.sf(., "LINESTRING") :
repeating attributes for all sub-geometries for which they may not be constant

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common warning with st_cast() when geometry lengths differ.

#| title: tranform, view and filter

redeploy_loc_pts <- redeploy_loc %>%
st_cast("POINT") %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning message:
In st_cast.sf(., "POINT") :
repeating attributes for all sub-geometries for which they may not be constant

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common warning with st_cast() when geometry lengths differ.

## Select redeployment location
Then filter out the points we want, this will change depending on your study site and what locations you want. We will transform the projection back to WGS 84 as this is likely what your gps or sonar will want. We will also add in a few columns to conform to GLATOS and OTN data standards and rearrange the column order.
```{r, results = 'hide'}
redeploy_sites <- buffer_rings_pts %>%
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: object 'buffer_rings_pts' not found

It's unclear if you intended to use 'buffer_rings_pts' from earlier script (Section 2) or if you meant to use 'redeploy_loc_pts' here; but this needs to be fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated this to redeploy_loc_pts as that is what the correct object should be - sorry about that.

title: "Estimate Detection Range for
Acoustic Telemetry Receivers"
author: Benjamin L. Hlina
date: "Updated: `r Sys.Date()`"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest hardcoding the date here to reflect when this document was actually edited. Otherwise, it simply reflects the installation date.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the hard coded the date here

@chrisholbrook
Copy link
Collaborator

completed in commit 6525afd

@chrisholbrook chrisholbrook merged commit 405a195 into dev Sep 20, 2024
@chrisholbrook
Copy link
Collaborator

chrisholbrook commented Sep 20, 2024

@benjaminhlina This is now live in 0.8.0. Thanks for the contribution! I took the liberty of making a few edits to code chunks that were throwing warnings and errors (mentioned in review) that I thought should be resolved. Feel free to submit another PR if you'd like to correct any of those changes. Also, note some overlap between your vignette and 'detection_range_handout.html'. I will be encouraging you and others to revisit the topic of detection range analysis with the goal of identifying some consensus best practice. Even as separate vignettes, though, these are valuable contributions.

@benjaminhlina
Copy link
Collaborator

Thanks @chrisholbrook, I have pulled the changes you have made to this that you merged with dev and agree that all the warnings need to be explained to the user. I also a agree that the save steps need to actually save and not error as the end-user will not know why it's erroring and appreciate you amending those steps as well as correcting the warnings.

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

Successfully merging this pull request may close these issues.

4 participants