feature: add range functionality to `fix_time` in `Task`

[gcroci2]

• added range functionality to fix_time
• adjusted times order in the signals and fix the tests
• wrapped together functions for tuples
• added tests for fix_time as a range

[gcroci2]

• #68 👈
• #65
• #64
• #63
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @gcroci2 and the rest of your teammates on Graphite

[gcroci2]

I realize that in the currently planned experiments these are not needed, but I can easily imagine a future application where they are. Is there any reason not to include the option already?

We should ask how realistic it is that a user might need them, from the experiments point of view. Otherwise, I would avoid implementing them until someone requests the specific feature. Imo, stim_time is something you want to have fixed and not random. fix_intensity is not even much used as a constant value, from what I understood, so no need of adding code lines and tests for something it will be barely used even in its basic form. stim_intensity is the only one I think it could be useful because we are basically using it as a range already (e.g., [0.8, 0.9, 1.0]), but then the con is that in cases in which you are interested in very low values and very high values (e.g., [0.2, 0.25, 0.3, 0.35, 0.9, 0.95]), you don't have a way to set this with a range, because also random numbers in between will be chosen.

So to conclude, I'd stick to what we have now, without implementing additional ranges. If you're not convinced I'd ask to the others first which ones make more sense :)

[DaniBodor]

I approved this PR because I think at this point it's fine either with or without my suggestions above, your call.

[DaniBodor]

I realize that in the currently planned experiments these are not needed, but I can easily imagine a future application where they are. Is there any reason not to include the option already?

We should ask how realistic it is that a user might need them, from the experiments point of view. Otherwise, I would avoid implementing them until someone requests the specific feature. Imo, stim_time is something you want to have fixed and not random. fix_intensity is not even much used as a constant value, from what I understood, so no need of adding code lines and tests for something it will be barely used even in its basic form. stim_intensity is the only one I think it could be useful because we are basically using it as a range already (e.g., [0.8, 0.9, 1.0]), but then the con is that in cases in which you are interested in very low values and very high values (e.g., [0.2, 0.25, 0.3, 0.35, 0.9, 0.95]), you don't have a way to set this with a range, because also random numbers in between will be chosen.

So to conclude, I'd stick to what we have now, without implementing additional ranges. If you're not convinced I'd ask to the others first which ones make more sense :)

I think that to make the code flexible to whatever a user wants to do (that is not ridiculous), I don't see why not to add the options. Given that you've now written a functions to do implement the random ranges and checks that the inputs are as expected, it doesn't add much code to apply it to other parameters as well. Your call though on how to proceed.

I do definitely agree with the point you make about high and low values. I guess strictly speaking this could also be true for other parameters, but much less likely. Formally speaking a user could also prefer to draw from a different distribution than uniform. I think both these points could be implemented without too much problems, but neither of these are within the scope right now.

[gcroci2]

I realize that in the currently planned experiments these are not needed, but I can easily imagine a future application where they are. Is there any reason not to include the option already?

We should ask how realistic it is that a user might need them, from the experiments point of view. Otherwise, I would avoid implementing them until someone requests the specific feature. Imo, stim_time is something you want to have fixed and not random. fix_intensity is not even much used as a constant value, from what I understood, so no need of adding code lines and tests for something it will be barely used even in its basic form. stim_intensity is the only one I think it could be useful because we are basically using it as a range already (e.g., [0.8, 0.9, 1.0]), but then the con is that in cases in which you are interested in very low values and very high values (e.g., [0.2, 0.25, 0.3, 0.35, 0.9, 0.95]), you don't have a way to set this with a range, because also random numbers in between will be chosen. So to conclude, I'd stick to what we have now, without implementing additional ranges. If you're not convinced I'd ask to the others first which ones make more sense :)

I think that to make the code flexible to whatever a user wants to do (that is not ridiculous), I don't see why not to add the options. Given that you've now written a functions to do implement the random ranges and checks that the inputs are as expected, it doesn't add much code to apply it to other parameters as well. Your call though on how to proceed.

I think we need to balance a bit between developing the most flexible framework, the best practices, and what is needed for the experiments. It is true that the project is at its beginning, but I'd try to be selective about how to spend time on the code-base from the beginning. For extending the ranges thing, atm I think there are more possibilities that the functionality would be not used or would need edits to be used than the contrary, so I'd avoid doing that unless explicitly requested. Same for a different distribution than uniform, which would be a very nice feature actually. If it will be requested, should be fast enough to be implemented anyway.

[gcroci2]

I approved this PR because I think at this point it's fine either with or without my suggestions above, your call.

Maybe it can be useful to ask someone what's the best practice in such cases? Not really important for the PR itself, but useful for us from the soft engineering point of view I think

[sonarcloud[bot]]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud