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.

Sign up for GitHub

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

Jump to bottom

Feature Request: Handle potential floating point issues by using function signif() #2134

Closed
millerg23 opened this issue Sep 27, 2023 · 3 comments · Fixed by #2212
Closed

Feature Request: Handle potential floating point issues by using function signif() #2134

millerg23 opened this issue Sep 27, 2023 · 3 comments · Fixed by #2212
Assignees
@millerg23
Labels
enhancement New feature or request programming

Comments

@millerg23
Copy link
Collaborator

Feature Idea

Whilst developing DAIDS grading criteria it was discovered that R is susceptible to floating point issues, and as such the grading metadata was updated to use the signif() function when comparing 2 variables.
ie AVAL > ANRHI was changed to signif(AVAL, 15) > signif(ANRHI, 15) the value of 15 is passed into function derive_vars_atoxgr_dir.
We need to look if any other occurences need a similar solution, for example function derive_var_anrind().

Also, should the number of signficant digits we use in admiral be stored in a general option, thus making it easier for a company/study to change in a central place? (Perhaps a separate issue).

Relevant Input

No response

Relevant Output

No response

Reproducible Example/Pseudo Code

No response
@millerg23 millerg23 added enhancement New feature or request programming labels Sep 27, 2023
@bms63
Copy link
Collaborator

bms63 commented Oct 8, 2023

@StefanThoma might be of interest to you

@StefanThoma
Copy link
Collaborator

Thanks, @bms63

@bms63
Copy link
Collaborator

bms63 commented Oct 24, 2023

We should link @StefanThoma blog post in the changelog pharmaverse/blog#92

@millerg23 millerg23 self-assigned this Nov 1, 2023
@millerg23 millerg23 linked a pull request Nov 1, 2023 that will close this issue
Closes #2134 add general option signif_digits #2212
Merged
14 tasks
bms63 added a commit that referenced this issue Nov 14, 2023
@millerg23 @bms63 @StefanThoma
Closes #2134 add general option signif_digits (#2212)
8a18e0e 
* 2134 add general option signif_digits

* 2134 fix SPELLING and STYLER

* 2134 address QC comments and update NEWS file

* 2134 fix LINTR and STYLER

* Apply suggestions from code review

Minor updates from QC.

Co-authored-by: Ben Straub <ben.x.straub@gsk.com>
Co-authored-by: StefanThoma <40463122+StefanThoma@users.noreply.github.com>

* Update vignettes/lab_grading.Rmd

---------

Co-authored-by: Ben Straub <ben.x.straub@gsk.com>
Co-authored-by: StefanThoma <40463122+StefanThoma@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@millerg23 millerg23

Labels
enhancement New feature or request programming
Projects
admiral (sdtm/adam, dev, ci, template, core)
Status: Archive
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Closes #2134 add general option signif_digits pharmaverse/admiral
3 participants
@bms63 @StefanThoma @millerg23