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

fwrite allows sep="" #5091

Merged
merged 4 commits into from
Aug 5, 2021
Merged

fwrite allows sep="" #5091

merged 4 commits into from
Aug 5, 2021

Conversation

ben-schwen
Copy link
Member

Closes #4817

@mattdowle mattdowle added this to the 1.14.1 milestone Aug 4, 2021
@MichaelChirico
Copy link
Member

can we benchmark any potential performance impact of those branches added in potentially tight loops?

I guess they could be optimized away, makes sense to test with/without compiler optimization on

@mattdowle
Copy link
Member

Yes good thought. I was just about to remove the branches...

@mattdowle
Copy link
Member

mattdowle commented Aug 5, 2021

Yes: I'd be surprised if there was any measurable difference.
The things that might be worth trying are bool instead of int and declaring sep and sepLen locally as const inside that deep loop in the parallel region. That way the const is making it crystal clear to the compiler that each thread doesn't need to check on every iteration to see if the global sep and sepLen has changed. Without const it has to cater for the possibility that another thread changes those globals. As we rely on for the failed global to stop a parallel region early upon error.

@mattdowle mattdowle merged commit c2b5ca2 into master Aug 5, 2021
@mattdowle mattdowle deleted the fwrite_blank branch August 5, 2021 00:40
@archgen
Copy link

archgen commented Jul 28, 2022

Hello, I am getting the below error when trying to set sep='' for fwrite.. I was wondering if you can direct me to how to utilise the added fwrite sep='' functionality?

Thanks very much for your help!
Matthew

Error in fwrite(eigenstrat_file$geno, sep = "", quote = FALSE, row.names = FALSE, :
is.character(sep) && length(sep) == 1L && nchar(sep) == 1L is not TRUE

R version 4.1.2 (2021-11-01)
Platform: x86_64-apple-darwin17.0 (64-bit)
Running under: macOS Big Sur 10.16

other attached packages:
data.table_1.14.2

@ben-schwen
Copy link
Member Author

@archgen
This feature has not made it to CRAN yet and is only available in the development version.

You can install the development version using data.table::update_dev_pkg()

@archgen
Copy link

archgen commented Jul 28, 2022

Hi @ben-schwen thanks for your quick reply! I tried data.table::update_dev_pkg() but got the following error Error: 'update_dev_pkg' is not an exported object from 'namespace:data.table'

@ben-schwen
Copy link
Member Author

@archgen Then data.table:::update_dev_pkg() should work.

@archgen
Copy link

archgen commented Jul 28, 2022

Thanks again @ben-schwen , unfortunately, I get the error

Error in get(name, envir = asNamespace(pkg), inherits = FALSE) :
object 'update_dev_pkg' not found

I'm running on a cluster if that's important to know..

Thanks again for your time and help!

sessionInfo()
R version 3.6.0 (2019-04-26)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Red Hat Enterprise Linux Server 7.7 (Maipo)

Matrix products: default
BLAS/LAPACK: /apps/software/OpenBLAS/0.2.18-GCC-5.4.0-2.26-LAPACK-3.6.1/lib/libopenblas_haswellp-r0.2.18.so

locale:
[1] LC_CTYPE=en_AU.UTF-8 LC_NUMERIC=C
[3] LC_TIME=en_AU.UTF-8 LC_COLLATE=en_AU.UTF-8
[5] LC_MONETARY=en_AU.UTF-8 LC_MESSAGES=en_AU.UTF-8
[7] LC_PAPER=en_AU.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_AU.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] data.table_1.14.2

loaded via a namespace (and not attached):
[1] compiler_3.6.0 tools_3.6.0

@jangorecki
Copy link
Member

jangorecki commented Jul 28, 2022

data.table:::update.dev.pkg()

The "_" function name is in 1.14.3+

@jangorecki jangorecki modified the milestones: 1.14.9, 1.15.0 Oct 29, 2023
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.

fwrite could allow sep=""
5 participants