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

Replace utils read with readr read function #442

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Replace utils read with readr read function #442

merged 1 commit into from
Oct 7, 2024

Conversation

r-ash
Copy link
Contributor

@r-ash r-ash commented Oct 2, 2024

Was just having a quick look at anc_spectrum_warning and art_spectrum_warning as we will use similar code to build the data for the new input plots. The majority of the time running these functions is just reading the spectrum files. Particularly for large countries. I've replaced the reading with utils::read.csv with readr as it is quite a bit faster.

For DRC, read_dp is the function before this change, read_dp2 is the function after this change. Note the only difference should be is that where read_dp would return columns with the types set by R. They are always "character" now. I believe this is fine because we have to manually convert any data later anyway (see e.g. read_dp_art_dec31) as the type interpretation is often broken due to how DP files are stored in disk.

bench::mark(read_dp(data$pjnz), read_dp2(data$pjnz), check = FALSE, iterations = 5)
# A tibble: 3 × 13
  expression             min median `itr/sec` mem_alloc `gc/sec` n_itr  n_gc total_time result memory     time       gc      
  <bch:expr>           <bch> <bch:>     <dbl> <bch:byt>    <dbl> <int> <dbl>   <bch:tm> <list> <list>     <list>     <list>  
1 read_dp(data$pjnz)   313ms  341ms      2.44    75.5MB     1.95     5     4      2.05s <NULL> <Rprofmem> <bench_tm> <tibble>
2 read_dp2(data$pjnz)  176ms  188ms      5.31    24.1MB     1.06     5     1   941.76ms <NULL> <Rprofmem> <bench_tm> <tibble>

@r-ash r-ash requested review from jeffeaton and rtesra October 3, 2024 08:24
Copy link
Collaborator

@jeffeaton jeffeaton left a comment

Choose a reason for hiding this comment

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

Nice spot. Thanks. This is something that might have benefit for changing across other packages used here (EPP, Shiny90, leapfrog, etc.)

Bigger rewrites could be done to avoid re-reading the DP file multiple times.

@r-ash r-ash merged commit 4cc15f4 into master Oct 7, 2024
8 checks passed
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.

2 participants