BrainardLab / OneLightToolbox

Brainard Lab code for talking to our OneLight boxes
MIT License
1 stars 0 forks source link

Only convert unique values OLPrimaryToSettings, OLSettingsToStartsStops #16

Closed JorisVincent closed 6 years ago

JorisVincent commented 6 years ago

When processing large numbers of primary values, OLPrimaryToSetting and OLSettingsToStartsStops take significant time. When the primary values form a periodic pattern (e.g. a sinewave), these functions perform duplicate calculations by converting the same primary value(s) multiple times.

Somewhere in the toolbox (I don't remember where), a clever little trick was used to only pass unique primary values to OLPrimaryToSettings, and then back-fill in the resulting settings. The same trick was used in providing only unique settings to OLPrimaryToStartsStops.

Is there any reason NOT to do this? And better yet, is it worth coding this speed-up trick into OLPrimaryToSetting and OLSettingsToStartsStops?

DavidBrainard commented 6 years ago

There isn't any reason not to do this, other than that one would need to write a good test program that compared the 'fast' way with the 'straightforward' way, and have that set up so we could easily run it in the future.

Perhaps good to have this as a key/value pair for the relevant functions.

JorisVincent commented 6 years ago

OLPrimaryToStartsStops now uses the 'fast' way. OLPrimaryToSettings and OLSettingsToStartsStops still convert the 'straightforward' way. Example in OLPrimaryToStartsStops shows that they give the same result.