Skip to content

206 optimize preprocess cdc function for speed - #208

Draft
truenomad wants to merge 36 commits into
devfrom
206-optimize-preprocess_cdc-function-for-speed
Draft

206 optimize preprocess cdc function for speed#208
truenomad wants to merge 36 commits into
devfrom
206-optimize-preprocess_cdc-function-for-speed

Conversation

@truenomad

Copy link
Copy Markdown
Collaborator

In this PR I managed to improve the speed of preprocess_cdc() by applying several refactorings. Most importantly, I moved st_make_valid() out of Step 2 and into process_spatial(), so validity checks only run once during preprocessing of new shapefiles.

As a result, the function now runs in just under 10, this for global POLIS data. The remaining bottlenecks are in Steps 2 and 5, where we load, save, and bind the large AFP datasets. These steps should speed up considerably if we decide to drop unnecessary raw columns and convert strings to factors, as we discussed recently @mcuadera.

I also cleaned up console output so that preprocess_cdc() produces concise, informative CLI messages rather than verbose logs or warnings.

Closes #206

@truenomad truenomad self-assigned this Jun 6, 2025
@truenomad truenomad added the enhancement New feature or request label Jun 6, 2025

@mcuadera mcuadera left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey Mo, added my feedback. For f.pre.st.sample.01(), we really need to make sure that we have the ability to filter for shapes that we were unable to make valid. This means modifying process_spatial() so that the outputs include a logical column indicating whether we were successful in making a shape valid. Maybe we can this "is_valid"

Comment thread R/dal.R
Comment thread DESCRIPTION
Comment thread R/dal.R Outdated
Comment thread R/init.R Outdated
Comment thread R/utils.R
Comment thread R/utils.R
Comment thread R/utils.R
@mcuadera

Copy link
Copy Markdown
Collaborator

Another idea to increase the speed is to write R data objects in parallel. Need to look at points in preprocessing where we're sequentially writing to disk or to EDAV, and then using future.

@truenomad truenomad left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Another idea to increase the speed is to write R data objects in parallel. Need to look at points in preprocessing where we're sequentially writing to disk or to EDAV, and then using future.

Yes, I think this can work. I haven't done this before, so look to your guidance.

Also, using qs2 and dropping already cleaned cols will make the key difference!

@mcuadera

Copy link
Copy Markdown
Collaborator

Another idea to increase the speed is to write R data objects in parallel. Need to look at points in preprocessing where we're sequentially writing to disk or to EDAV, and then using future.

Yes, I think this can work. I haven't done this before, so look to your guidance.

Also, using qs2 and dropping already cleaned cols will make the key difference!

Indeed, and qs2 has the rds_to_qs() function as well that we could potentially use. Thinking about when we pull data fom POLIS, which come as RDS as of now (maybe there should be an output type param for that as well)

@truenomad

truenomad commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator Author

@mcuadera actually, looking into this more closely, it might be easier to simply specify the output_format directly within get_polis_data, since importing raw data contributes to preprocessing speed. We’ve already done the heavy lifting, tidypolis_io can read and write qs2 files. We just need to make sure output_format is specified in the function, which should be straightforward.

Let me try that now!

Update I have been able to update get_polis_data as well s1_prep_polis_tables. In s1_prep_polis_tables I have separate output_format from raw_output_format so that we the saving format and raw polis data format are different. I hope this is within scope!

@mcuadera mcuadera linked an issue Jun 24, 2025 that may be closed by this pull request
@mcuadera
mcuadera removed the request for review from Heaghneyn July 2, 2025 15:37
mcuadera added 3 commits July 2, 2025 12:43
Updated the logic for type checking in get_polis_data to use setdiff for more robust handling of 'all' and 'pop' types. Also improved error messaging for invalid types.
@mcuadera

Copy link
Copy Markdown
Collaborator

oof...tried to merge dev into this branch and encountered too many conflicts. I think I will be cherry picking from this branch into a newer branch just to make sure that I don't inadvertently do a bad merge

@mcuadera
mcuadera marked this pull request as draft December 22, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

❇️ Optimize preprocess_cdc() function for speed

2 participants