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

Some slight speedups of convert units #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Carter12s
Copy link

While running the "help_functions_chemdose_ph" vignette I started profiling some of the code as I noticed it was taking a while to run on my computer. Specifically I was profiling this snippet with profvis

profvis({
          solve_column <- raw_water %>%
                  chemdose_ph_chain(input_water = "balanced_water") %>%
                 cross_join(raise_ph) %>%
                  solvedose_ph_once(input_water = "dosed_chem_water") %>%
                 select(-c(defined_water:dosed_chem_water))   
          })

The baseline case on main was taking ~7.5 seconds to run on my machine:
image
With convert_units accounting for ~2.1 seconds of runtime. Although results are very messy.

After these optimizations the example is down to ~7.0 seconds, and convert_units is down to 1.9 seconds. Not huge gains, but a start.
image

Going to continue to optimize from here, but wanted to at least show these gains were possible.

@Carter12s
Copy link
Author

Okay I've applied a memoization approach of pre-computing the standard unit conversions and then looking them up instead of recalculating each time.

image

This results in the same example taking 5.8s on my machine (~22% faster) and convert_units is now down to 800ms aggregate time instead of 2.1s (~61% faster).

I'm less familiar with how to distribute R data files, but it does look like storing a table of pre-computed unit conversions will be a significant speedup to the overall package.

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.

1 participant