Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

raise warning when no external drives added #269

Closed
cjayb opened this issue Feb 11, 2021 · 6 comments · Fixed by #516
Closed

raise warning when no external drives added #269

cjayb opened this issue Feb 11, 2021 · 6 comments · Fixed by #516
Labels
docs Extra attention is needed enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@cjayb
Copy link
Collaborator

cjayb commented Feb 11, 2021

@cjayb could we perhaps raise a warning when running a simulation with no input drives? And telling users that they must add the drives manually or use add_drives_from_params=True. If they use add_drives_from_params=True, a big fat deprecation warning should be raised.

+1 on both counts.

perhaps we could not add a drive to net.external_drives if in the param file t0 of drive > tstop of simulation?

and

The items in net.external_drives should map 1-to-1 with whatever is specified by a param file or the user via calls to net.add_... so that we don't end up silently loosing track of network drives, gids, etc.

I propose raising a ValueError for drives (from params or otherwise) where t0 > T. The solution is either to fix the param-file (setting weights to zero effectively "removes" the drive), or, preferably, use the new API.

@cjayb cjayb added docs Extra attention is needed enhancement New feature or request labels Feb 17, 2021
@jasmainak jasmainak added this to the 0.2 milestone Mar 5, 2021
@jasmainak
Copy link
Collaborator

I propose raising a ValueError for drives (from params or otherwise) where t0 > T

I think we agreed in #397 that we'll truncate rather than raise an error?

The warning is easy to tackle?

I'm trying to look at low hanging fruits from 0.2 milestone and get them done to prepare for a release

@jasmainak
Copy link
Collaborator

probably easily bunched with #410 ?

@cjayb
Copy link
Collaborator Author

cjayb commented Sep 3, 2021

Right, now that tstop is a parameter of the simulation, the t0 of the drives should be allowed to be arbitrary. Perhaps a RuntimeError if simulate_dipole is called on a net without drives?

@jasmainak
Copy link
Collaborator

yeah that sounds good. Not sure if we want an error or warning ... I remember @ntolley saying there were situations where you'd want to simulate a network without driving it

@jasmainak jasmainak modified the milestones: 0.2, 0.3 Sep 28, 2021
@jasmainak jasmainak added the good first issue Good for newcomers label Feb 10, 2022
@ntolley
Copy link
Contributor

ntolley commented Jun 14, 2022

Close issue? I think we've moved towards the opinion that less warnings is good, but maybe this one is worth it.

@jasmainak
Copy link
Collaborator

I would say it's worth it and an easy issue to tackle for a new contributor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Extra attention is needed enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants