Refactor/perf batch 8

[timcadman]

Background

Batch 8 of the performance refactor covers the random-number and sampling functions: ds.rBinom, ds.rNorm, ds.rPois, ds.rUnif, ds.sample and ds.setSeed. It moves object checks to the server, replaces eval(parse()) with safe object loading and removes the extra MODULE 5 checks.

In the process of refactor, whilst switching pasting text to call(), it became clear that despite the docs stating that each study would be passed its own values from arguments size, prob, mean, sd, force.output.to.k.decimal.places, the code never actually did this. This has been fixed: see table below for change in behaviour.

Specific changes to fix this silent code failure

This table summarises the previous behaviour and the new behaviour:

Input (3 studies)	Before: sent to every study	Now: sent to study 1 / 2 / 3
mean = 5	rNormDS(50,5,1,9)	mean = 5 / 5 / 5
mean = c(1, 6, 4)	rNormDS(50,1,1,9) (only the first value, silently)	mean = 1 / 6 / 4
mean = c(1, 6) (wrong length)	rNormDS(50,1,1,9) (only the first value, silently)	Nothing sent; error 'mean' must be length 1 or one value per study
mean = "D$LAB_TSC" (server-side name)	rNormDS(50,D$LAB_TSC,1,9), evaluated on the server	mean = "D$LAB_TSC" / same / same, loaded safely
mean = c("m1", "m2", "m3") (one name per study)	rNormDS(50,m1,1,9) (only the first name)	mean = "m1" / "m2" / "m3"
mean = "5" (number as text)	rNormDS(50,5,1,9), run as the number 5	mean = "5", which errors on the server: '5' does not exist
mean = "D$x*2" (expression as text)	rNormDS(50,D$x*2,1,9), run as code	mean = "D$x*2", which errors on the server: object not found
samp.size = c(10, 20, 45)	rNormDS(10,...) / rNormDS(20,...) / rNormDS(45,...)	Unchanged: 10 / 20 / 45

The same applies to:

• ds.rBinom: size, prob
• ds.rNorm: mean, sd, force.output.to.k.decimal.places
• ds.rPois: lambda
• ds.rUnif: min, max, force.output.to.k.decimal.places

Standard changes as part of refactor

• Server (dsBase):
  • rBinomDS, rNormDS, rPoisDS, rUnifDS and sampleDS load object-name arguments with .loadServersideObject(); a missing object errors with "does not exist".
  • setSeedDS reads the seed with as.integer() instead of eval(parse()).
• Client (dsBaseClient):
  • server calls use call() instead of pasted text
  • datasources are resolved with .set_datasources()
  • the MODULE 5 checks are removed

Tests added:

• dsBase: tests for missing objects, and for setSeedDS with "NULL" and non-numeric seeds.
• Client smoke tests: missing objects, per-study values, and a wrong-length vector.
• New test-arg-per-study-arguments.R (no server needed): checks exactly what each study is sent.
• discctrl assertions: updated for the new call format.
• Perf tests: added.
• Docs: @return corrected, @param states the per-study rule, and the examples use numeric vectors instead of as.character(c(...)).

How to test

• Check CI green
• Review change in behaviour to fix previously silently failing code

[github-actions]

⏳ 4 of 5 checks reported so far

Check	Result
Devtools checks	✅ passed
Code quality	✅ 0 new findings (package total: 2107)
Armadillo unit tests	✅ [ FAIL 0 | WARN 0 | SKIP 27 | PASS 11186 ]
Opal unit tests	✅ [ FAIL 0 | WARN 0 | SKIP 27 | PASS 11208 ]
Test coverage	ℹ️ no coverable lines changed (project: 61.2%)

Tested against dsBase versions:
Armadillo: 7.0.0.9000
Opal: 7.0.0.9000

Logs: Devtools checks · Code quality · Armadillo unit tests · Opal unit tests · Codecov

[github-advanced-security]

lintr found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.16%. Comparing base (e3221f5) to head (7cecae2).

Additional details and impacted files

@@             Coverage Diff              @@
##           v7.0-dev     #702      +/-   ##
============================================
+ Coverage     60.87%   61.16%   +0.28%     
============================================
  Files           142      142              
  Lines          6365     6120     -245     
============================================
- Hits           3875     3743     -132     
+ Misses         2490     2377     -113     

Flag	Coverage Δ
armadillo-arg	21.01% <ø> (+3.00%)	⬆️
armadillo-misc	15.98% <ø> (+0.56%)	⬆️
armadillo-perf-1	4.33% <ø> (+0.16%)	⬆️
armadillo-perf-2	13.15% <ø> (+0.50%)	⬆️
armadillo-perf-3	9.42% <ø> (+3.39%)	⬆️
armadillo-smk-1	29.82% <ø> (+1.14%)	⬆️
armadillo-smk-2	37.02% <ø> (-0.84%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[StuartWheater]

• "Failure ('test-perf-ds.Boole.R:40:5'): performance" issue is addressed by stage 9/10.
• Changes to code and tests appear correct, both restructuring and code tweeks