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

Improve performance of .lvCompare() #425

Draft
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

Qile0317
Copy link
Collaborator

@Qile0317 Qile0317 commented Oct 23, 2024

A general improvement on the runtime of combineBCR() by accelerating .lvCompare() with rcpp by a small constant factor.

@Qile0317 Qile0317 marked this pull request as ready for review October 23, 2024 01:22
@Qile0317
Copy link
Collaborator Author

@ncborcherding Just realized that the vignette build failed in the R-CMD-check with the following:

Error: Error: processing vignette 'vignette.Rmd' failed with diagnostics:
object 'uni_IG' not found
--- failed re-building ‘vignette.Rmd’

and also

The magick package is required to crop "/tmp/Rtmp7YAETR/Rbuild179452a8e5c7/scRepertoire/vignettes/vignette_files/figure-html/unnamed-chunk-55-1.png" but not available.
Quitting from lines 1146-1159 [unnamed-chunk-57] (vignette.Rmd)

Will get on that later. Unsure why vignette is failing since it wasn't really touched.

@ncborcherding
Copy link
Owner

Looks like the clonalCluster() is broken or at least the internal .lvCompare() based on the error message.

I think the magick warning is a BiocStyle issue.

@Qile0317 Qile0317 marked this pull request as draft October 24, 2024 22:44
@Qile0317 Qile0317 changed the title Improve performance of .lvCompare() Improve performance of .lvCompare() Oct 29, 2024
@Qile0317 Qile0317 self-assigned this Oct 31, 2024
@Qile0317 Qile0317 added enhancement New feature or request help wanted Extra attention is needed labels Oct 31, 2024
})
cluster <- bind_rows(cluster.list)
cluster <- bind_rows(cluster.list) # the TRA_cluster isnt assigned in the failing test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ncborcherding Could you maybe describe what exactly this intermediate variable is supposed to be? Struggling to debug downstream atm.

Copy link
Owner

Choose a reason for hiding this comment

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

@Qile0317 Here the cluster.list is the cluster assignments from the .lv.compare() function that are grouped into list elements, bind_rows() is just forming a data frame from the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants