Skip to content

Conversation

@Remi-Gau
Copy link
Contributor

@Remi-Gau Remi-Gau commented Aug 18, 2020

@marcobarilari

Opening this one already so you can follow along and make sure my logic is not out of whack.

Doing it again. This time with CI.

@Remi-Gau Remi-Gau linked an issue Aug 18, 2020 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Aug 22, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@ede2bb5). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##             master     #38   +/-   ##
========================================
  Coverage          ?   0.00%           
========================================
  Files             ?       3           
  Lines             ?     126           
  Branches          ?       0           
========================================
  Hits              ?       0           
  Misses            ?     126           
  Partials          ?       0           
Flag Coverage Δ
#unittests 0.00% <0.00%> (?)

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


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 ede2bb5...11b3ae5. Read the comment docs.

@Remi-Gau
Copy link
Contributor Author

Currently trying to fix some CI issues but for information the trial randomization was getting caught in a loop because we had one check that made sure that a target was not presented more than 3 times in a given position which becomes impossible when you have a large number of blocks.

@Remi-Gau Remi-Gau linked an issue Aug 22, 2020 that may be closed by this pull request
@Remi-Gau Remi-Gau requested a review from marcobarilari August 22, 2020 09:20
@Remi-Gau
Copy link
Contributor Author

@marcobarilari let me know what you think.

Once this is OK we can apply the changes to the audio localizer

@Remi-Gau Remi-Gau linked an issue Aug 22, 2020 that may be closed by this pull request
@marcobarilari
Copy link
Collaborator

On a side: I am tempted to move out the code for displaying the design in an image in a different function taking cfg as input. Because right now, without the possibility to input the params inside this function, is kind of useless. Even though you run the experiment with the input to display the image, they are close when the experiment ends.

LMK

@marcobarilari
Copy link
Collaborator

I attach here the plots from one run of the experiment in this branch. Should we control for a better distribution of the targets across the events?

You can see that in fig 1 plot 3 many targets are in event 2

Screen Shot 2020-08-23 at 15 43 43

@Remi-Gau
Copy link
Contributor Author

On a side: I am tempted to move out the code for displaying the design in an image in a different function taking cfg as input. Because right now, without the possibility to input the params inside this function, is kind of useless. Even though you run the experiment with the input to display the image, they are close when the experiment ends.

LMK

good point !!

Can I let you do that ?

@Remi-Gau
Copy link
Contributor Author

I attach here the plots from one run of the experiment in this branch. Should we control for a better distribution of the targets across the events?

You can see that in fig 1 plot 3 many targets are in event 2

I am not too obsessed with this kind of thing on a localizer that we will use for one run per participant. Also because random means that you might have that kind of clustering once in a while.

So this would be low priority at the moment for me.

You could expand the unit test to ensure that over a large amount of runs we do get a uniform distribution of the targets even if on a given run you can have this kind of clustering.

If you want to tweak things to enforce a more uniform distribution on every run, you have to change the conditions to rule 3 (line 104 in expDesign) but then make sure you also change it in its unit test line 39 and 73.

Just remember that this condition has to scale with number of blocks we have in a run and that this was the line that was creating infinite loops because it would fail to find a solution.

@Remi-Gau
Copy link
Contributor Author

trying to fix the merge conflixt on the submodule. Grrr...

@Remi-Gau Remi-Gau merged commit 7abbfe6 into cpp-lln-lab:master Aug 24, 2020
@Remi-Gau Remi-Gau deleted the remi-fix_randomize branch August 24, 2020 06:09
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.

improve expDesign make flexible which direction to show in setParameters Set up actions to check code standard

2 participants