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

bird insect partitioning and post-hoc precipitation filter #698

Merged
merged 32 commits into from
Mar 20, 2025

Conversation

@adokter adokter added this to the 0.9 milestone Feb 26, 2025
@adokter adokter self-assigned this Feb 26, 2025
@adokter adokter marked this pull request as draft February 26, 2025 21:26
@adokter adokter changed the title partitioning of bird insect mixtures bird insect partitioning and post-hoc precipitation filter Feb 27, 2025
@adokter adokter requested a review from peterdesmet February 27, 2025 14:41
@adokter adokter marked this pull request as ready for review February 27, 2025 15:00
@adokter
Copy link
Owner Author

adokter commented Mar 4, 2025

@bart1 can you please review if this PR fixes your bug report on duplicate timestamps (#697) and close that issue if it has

@adokter adokter requested a review from bart1 March 4, 2025 18:10
@bart1
Copy link
Collaborator

bart1 commented Mar 4, 2025 via email

@adokter adokter merged commit 8f12bb1 into master Mar 20, 2025
5 checks passed
@adokter
Copy link
Owner Author

adokter commented Mar 20, 2025

@bart1 decided to merge, as this PR had some critical fixes I wanted posted to master asap. Feel free to add your review here nonetheless and we can merge those changes later

@bart1
Copy link
Collaborator

bart1 commented Mar 21, 2025

@adokter Sorry for the delay, I just start working a bit with this, somethings I noticed below

First of all nice changes, useful to make script easier! On the topic of correcting VPTS data, maybe we could also think about filling holes in a vpts when the sdvvp drops to low during intense migration. That is a other frequent fix I used.

not equally high profiles

I noticed as.vpts requires the height profiles to be exactly equal otherwise it drops the profiles. If the height interval is equal (e.g. 200 meters) it might be nice to just NA fill the profiles? I think the aloft profiles for example not always go up to the same height:

devtools::load_all("~/bioRad/")
#> ℹ Loading bioRad
#> Welcome to bioRad version 0.8.1.9000
#> 
#> using vol2birdR version 1.0.5 (MistNet not installed)
pvolfile_source <- system.file("extdata", "volume.h5", package = "bioRad")

pvolfile <- paste0(tempdir(),"/volume.h5")
file.copy(pvolfile_source, pvolfile)
#> [1] TRUE

  vp <- calculate_vp(pvolfile) |> as.data.frame()
#> Running vol2birdSetUp
#> Warning: radial velocities will be dealiased...
  vp_low <- calculate_vp(pvolfile, n_layer = 10)|> as.data.frame()
#> Running vol2birdSetUp
#> Warning: radial velocities will be dealiased...
  
as.vpts(rbind(vp, vp))
#> Warning in validate_vpts(data): Extra fields found: day, sunrise, sunset
#>                    Regular time series of vertical profiles (class vpts)
#> 
#>            radar:  seang 
#>       # profiles:  2 
#> time range (UTC):  2015-10-18 18:00:00 - 2015-10-18 18:00:00 
#>    time step (s):  0
as.vpts(rbind(vp, vp, vp_low))
#> Warning in validate_vpts(data): Extra fields found: day, sunrise, sunset
#> Warning in as.vpts(rbind(vp, vp, vp_low)): Profiles found with different altitude layer specifications, keeping only the most
#>     common one, dropping  1 profile(s) ...
#> Warning in as.vpts(rbind(vp, vp, vp_low)): Profiles found with different number
#> of height layers: 10 20, retaining 20 only.
#>                    Regular time series of vertical profiles (class vpts)
#> 
#>            radar:  seang 
#>       # profiles:  2 
#> time range (UTC):  2015-10-18 18:00:00 - 2015-10-18 18:00:00 
#>    time step (s):  0

Here it would be nice if vp_low was just filled with NA values especially if the difference is in the high range (e.g. a mixture of profiles upto 4000 and 5000 meter)

Clean mixture

I would watch out with the argument naming u vs U, and v vs V, would it not be better to use something like V_wind. I know most of the time it won't be needed but still I could see bugs slipping in easily. I find capitalization to distinguish parameters difficult.

Filter precip

Maybe you can use something like this as a default parameter for dbz : ifelse(x$attributes$how$wavelength<7,x, y). In this way you can optimize the default for both C and S band, that should at least help with the last note in the help description.

#697

Is mostly resolved except the source_file seems to be mixed up. Not a big issue as I expect not many people to use it but still it would be good to make sure that is correct

@adokter
Copy link
Owner Author

adokter commented Mar 24, 2025

Thanks @bart1, I'll start working on incorporating this feedback

@adokter
Copy link
Owner Author

adokter commented Mar 24, 2025

Incorporating reviewer feedback to this PR in #704

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.

Duplicated timestamps in csv change height distribution in vpts plot
3 participants