Skip to content

ncanda-operations calculations have changed precision #22

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

Open
shippy opened this issue Oct 10, 2018 · 16 comments
Open

ncanda-operations calculations have changed precision #22

shippy opened this issue Oct 10, 2018 · 16 comments
Assignees

Comments

@shippy
Copy link
Contributor

shippy commented Oct 10, 2018

For LEQ and AEQ scores, previous pandas version resulted in floats with 17 decimal places. Current pandas version outputs 13 decimal places. This makes Redcap think that the calculation outcome has changed, which results in a large number of errors.

Possible solutions:

  • Bring precision back to 17
  • Create an explicitly setting that rounds the calculation to n decimal places (per @kipohl, preferred n = 8) and update the scores
@kipohl
Copy link
Contributor

kipohl commented Oct 11, 2018

Error:post_github_issues: Failed to post the following issue on github! Title: leq-6e38e7, session:redcap_import_record:Failed to import into REDCap, Body: ### NCANDA REDCap (redcap_update_summary_scores)

  • requestError: {u'error': u'"A-00002-F-2","leq_c_nu","0.0454545454545","This field is located on a form that is locked. You must first unlock this form for this record."\n"A-00002-F-2","leq_c_u","0.0384615384615","This field is located on a form that is locked. You must first unlock this form for this record."\n"A-00002-F-2","leq_c_nc","0.0769230769231","This field is located on a form that is locked. You must first unlock this form for this record."\n"A-00002-F-2","leq_c_c","0.0512820512821","This field is located on a form that is locked. You must first unlock this form for this record."\n"A-00002-F-2","leq_c_sn","0.0571428571429",

@kipohl
Copy link
Contributor

kipohl commented Oct 11, 2018

Replicate error by running
/sibis-software/python-packages/sibispy/tests$ ./test_redcap_compute_summary_scores.py --id-list E-01335-M-6 --form-list leq,aeq,bmi, --uploadScores ~/.sibis-general-config.yml

Once you have confirmed that the fields with 18 digits are properly reduced to 8 digits than I will approve the pull request

@kipohl
Copy link
Contributor

kipohl commented Oct 11, 2018

note I added bmi to the test

@kipohl
Copy link
Contributor

kipohl commented Oct 11, 2018

@jimklo if possible please also include the option in the test script that you can compare the scored output with a previous output saved to file with the --dir option

@jimklo
Copy link
Contributor

jimklo commented Oct 11, 2018

@kipohl and @shippy
So interesting problem... when I run test with unaltered leq and aeq instruments, I get the 17 places of precision.

Conversely, when I go and set the precision for pandas to 8 it's ignored. From what I can tell the setting is a hint, and not necessarily guaranteed. I'm also not 100% sure that the precision shouldn't not be configured within the file write operation per https://pandas.pydata.org/pandas-docs/stable/options.html

@kipohl
Copy link
Contributor

kipohl commented Oct 11, 2018

please just use the round function (to the 8th digit) for those variables - that should take care of the issue

@jimklo
Copy link
Contributor

jimklo commented Oct 12, 2018

@kipohl is this the kind of output you were expecting? it's not exactly fixed precision rounding.

ncanda@sibis-storage[]:/sibis-software/python-packages/sibispy/tests$ cat /fs/ncanda-share/beta/junk/leq_A-00002-F-2_out.csv
study_id,redcap_event_name,leq_complete,leq_c_dnu,leq_c_cnu,leq_c_dau,leq_c_dnc,leq_c_cnc,leq_c_dpc,leq_c_nu,leq_c_dcu,leq_c_u,leq_c_nc,leq_c_c,leq_c_sn
A-00002-F-2,2y_visit_arm_1,1,0.0625,0.0,0.0,0.125,0.0,0.8,0.04545455,0.05,0.03846154,0.07692308,0.05128205,0.05714286
A-00002-F-2,1y_visit_arm_1,1,0.0,0.0,0.0,0.0,0.0,0.6,0.0,0.0,0.0,0.0,0.0,0.0
A-00002-F-2,4y_visit_arm_1,1,0.125,0.0,0.0,0.0,0.0,0.0,0.09090909,0.1,0.07692308,0.0,0.05128205,0.05714286
A-00002-F-2,3y_visit_arm_1,1,0.0,0.0,0.0,0.0,0.0,0.4,0.0,0.0,0.0,0.0,0.0,0.0
A-00002-F-2,baseline_visit_arm_1,1,0.0,0.0,0.0,0.25,0.0,0.8,0.0,0.0,0.0,0.15384615,0.05128205,0.05714286

and

ncanda@sibis-storage[]:/sibis-software/python-packages/sibispy/tests$ cat /fs/ncanda-share/beta/junk/aeq_A-00002-F-2_out.csv
study_id,redcap_event_name,aeq_complete,aeq_cmi,aeq_gpc,aeq_se,aeq_icma,aeq_ia,aeq_csb,aeq_rtr,aeq_score
A-00002-F-2,2y_visit_arm_1,1,4.0,1.66666667,2.0,1.0,1.66666667,1.66666667,2.66666667,1.80952381
A-00002-F-2,1y_visit_arm_1,1,3.33333333,2.66666667,2.0,2.0,3.33333333,2.0,4.0,2.66666667
A-00002-F-2,4y_visit_arm_1,1,4.33333333,2.33333333,3.33333333,1.33333333,2.66666667,4.0,4.0,2.76190476
A-00002-F-2,3y_visit_arm_1,1,4.0,1.0,1.0,1.0,1.0,1.0,1.33333333,1.19047619
A-00002-F-2,baseline_visit_arm_1,1,3.66666667,2.33333333,2.66666667,1.33333333,3.33333333,3.0,3.66666667,2.66666667

@kipohl
Copy link
Contributor

kipohl commented Oct 12, 2018 via email

@jimklo
Copy link
Contributor

jimklo commented Oct 12, 2018

I don't have the aeq version anymore, but for leq the original was:

study_id,redcap_event_name,leq_complete,leq_c_dnu,leq_c_cnu,leq_c_dau,leq_c_dnc,leq_c_cnc,leq_c_dpc,leq_c_nu,leq_c_dcu,leq_c_u,leq_c_nc,leq_c_c,leq_c_sn
A-00002-F-2,2y_visit_arm_1,1,0.0625,0.0,0.0,0.125,0.0,0.8,0.045454545454545456,0.05,0.038461538461538464,0.07692307692307693,0.05128205128205128,0.05714285714285714
A-00002-F-2,1y_visit_arm_1,1,0.0,0.0,0.0,0.0,0.0,0.6,0.0,0.0,0.0,0.0,0.0,0.0
A-00002-F-2,4y_visit_arm_1,1,0.125,0.0,0.0,0.0,0.0,0.0,0.09090909090909091,0.1,0.07692307692307693,0.0,0.05128205128205128,0.05714285714285714
A-00002-F-2,3y_visit_arm_1,1,0.0,0.0,0.0,0.0,0.0,0.4,0.0,0.0,0.0,0.0,0.0,0.0
A-00002-F-2,baseline_visit_arm_1,1,0.0,0.0,0.0,0.25,0.0,0.8,0.0,0.0,0.0,0.15384615384615385,0.05128205128205128,0.05714285714285714

FWIW: I've not tried the upload yet, but am focused more on making sure I understand what you wanted. The aeq version basically was similar but the the precision going out to 17 places (it wasn't 13 for me)

@kipohl
Copy link
Contributor

kipohl commented Oct 12, 2018

sure look good to me - why is this is not exactly fixed prevision rounding ?

@jimklo
Copy link
Contributor

jimklo commented Oct 12, 2018

@kipohl granted I don't know the the precision of the source values are - however when someone says they want precision of 8, i'm thinking 8 sig figs (my background is originally an architect - so physics is kind of ingrained in the brain). So for all these values < 1, they should all have at least 8 places right of the decimal. Rounding to 8 is just different... and going back and looking at the top of the page... I'm now realizing that @shippy did include "rounding" so yeah this is fine.

@jimklo
Copy link
Contributor

jimklo commented Oct 12, 2018

oh and I've always interpreted 'fixed" precision of n decimal points as more an artificial extension.... e.g. for n=8 2.0 becomes 2.00000000 which is theoretically incorrect as it expresses a value overly precise.

@kipohl
Copy link
Contributor

kipohl commented Oct 12, 2018

the way you have it is great no worry about fixed precision

@jimklo
Copy link
Contributor

jimklo commented Oct 12, 2018

tried upload aeq got the following error, but I thing we expected this since I think you said it has to be unlocked?

{"experiment_site_id": "aeq-495afc", "error": "session:redcap_import_record:Failed to import into REDCap", "requestError": "{u'error': u'\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_cmi\",\"4.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_gpc\",\"1.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_se\",\"2.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_icma\",\"1.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_ia\",\"1.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_csb\",\"1.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_rtr\",\"2.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (2y_visit_arm_1)\",\"aeq_score\",\"1.80952381\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_cmi\",\"3.33333333\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_gpc\",\"2.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_se\",\"2.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_icma\",\"2.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_ia\",\"3.33333333\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_csb\",\"2.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_rtr\",\"4.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (1y_visit_arm_1)\",\"aeq_score\",\"2.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_cmi\",\"3.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_gpc\",\"2.33333333\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_se\",\"2.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_icma\",\"1.33333333\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_ia\",\"3.33333333\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_csb\",\"3.0\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_rtr\",\"3.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"\\n\"A-00002-F-2 (baseline_visit_arm_1)\",\"aeq_score\",\"2.66666667\",\"This field is located on a form that is locked. You must first unlock this form for this record.\"'}", "red_api": "data_entry"}

@kipohl
Copy link
Contributor

kipohl commented Oct 12, 2018

that is expected - I am going offline now

@shippy
Copy link
Contributor Author

shippy commented May 7, 2019

Possibly related: the only change in Import project as a result of harvester -a was on E-99995-T-9-2012-12-13 PASAT: pasat_conpre changed from 2 to 2.0.

It might not be related, though: pandas is just notoriously bad at silently converting integers to floats, especially during .to_csv (which redcap.Project.import_records silently calls on data frames prior to upload).

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

No branches or pull requests

3 participants