Skip to content

Conversation

@marcobarilari
Copy link
Collaborator

not many info online for a best practice, anyway I think that this two are the main source to understand what is happening:

(1) Replace Discouraged Syntaxes of rand and randn

(2) ClockRandSeed

as you see there is a func from PTB but I think that in matlab might be better to use RandStream.setGlobalStream. From what I grasp is the best solution.

fix #95

@codecov
Copy link

codecov bot commented Sep 28, 2020

Codecov Report

Merging #113 into dev will decrease coverage by 0.67%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #113      +/-   ##
==========================================
- Coverage   26.10%   25.42%   -0.68%     
==========================================
  Files          43       44       +1     
  Lines         636      653      +17     
==========================================
  Hits          166      166              
- Misses        470      487      +17     
Flag Coverage Δ
#unittests 25.42% <0.00%> (-0.68%) ⬇️

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

Impacted Files Coverage Δ
src/utils/setUpRand.m 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6172baf...323259e. Read the comment docs.

@Remi-Gau
Copy link
Contributor

Oh nice.

I had gone the lazy way in this PR #105 and was reusing Sam Schwartzkopf implementation.

As the file is lost in the middle of this PR that covers almost" all but the kitchen sink", you can see the file itself on one of my branch here.

See maybe if there is anything you can take from there.

You function looks better refactored but I think Sam's covers VERY old matlab cases too.

@marcobarilari
Copy link
Collaborator Author

uh nice as well yours, I missed that otherwise I would have took in account, let me then mix them up.

Though, true that it covers also older matlab versions but it contradicts a bit what we check in our initEnv func.

Question is, how old should be your matlab to use the old rand seeders + that old version is ok with the rest of the CPP_PTB/BIDS repo.

@Remi-Gau
Copy link
Contributor

I think that having functions that support older matlab / octave is not a bad idea for something like this small function (that could be reused by others).

I think that to make our life easier we say that we will not "support" versions older than X is more for things where we would have to modify a whole bunch of function of with some if Ver > X; else because of a new feature in Octave / Matlab.

Does that make sense?

@Remi-Gau Remi-Gau merged commit 199069e into cpp-lln-lab:dev Sep 30, 2020
@marcobarilari marcobarilari deleted the marco_setUpRand branch September 30, 2020 09:47
@Remi-Gau Remi-Gau mentioned this pull request Oct 2, 2020
@Remi-Gau
Copy link
Contributor

Remi-Gau commented Oct 2, 2020

@marcobarilari we did not add the call to setUpRand in initPTB, right?

@marcobarilari
Copy link
Collaborator Author

yes we did not, doing now

marcobarilari added a commit to marcobarilari/CPP_PTB that referenced this pull request Oct 2, 2020
marcobarilari added a commit that referenced this pull request Oct 2, 2020
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

Successfully merging this pull request may close these issues.

create function to initialize RNG seed - must work both for matlab (including "old" versions) AND octave

2 participants