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

Use cl to calculate parallel #48

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Use cl to calculate parallel #48

merged 2 commits into from
Jul 30, 2024

Conversation

wleoncio
Copy link
Member

Since cl only makes sense if parallel = TRUE, it can be used to calculate the latter. This results in less function complexity (one fewer argument for the user to have to set), but I wonder if it makes the function any more intuitive.

If the user sets par = TRUE, cl is automatically set to 1. Otherwise, it defaults to detectCores() - 1L (instead of the previous 4, which causes issues on simpler systems such as remote runners).

Looking forward to your thoughts on this, Theo!

Merging this should close #40.

Since `cl` only makes sense if `parallel = TRUE`, the latter can be
replaced with the former (see complete discussion on #40).
@wleoncio wleoncio added the enhancement New feature or request label Jul 30, 2024
@wleoncio wleoncio requested a review from Theo-qua July 30, 2024 11:43
@Theo-qua
Copy link
Collaborator

Dear Waldir,
I think that makes sense. I did not consider it to be an issue initially. We can allow the user to specify this use or set the default to detectCores()-- 1L if that will not cause any problem.

Best regards,
Theo

@Theo-qua Theo-qua merged commit 55d8da9 into main Jul 30, 2024
7 checks passed
@wleoncio wleoncio deleted the issue-40 branch July 30, 2024 11:52
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.

Replace parallel with cl?
2 participants