[MRG] Set legacy mode to false by default

[ntolley]

closes #526

Changes all network models to use legacy_mode=False by default. Some minor changes in tests/examples were necessary but overall a pretty small change.

[ntolley]

Unfortunately #614 didn't solve all our problems since the GUI has it's own code to loop over drives and add them. I've added the same logic to skip over these drives.

Not really sure what the best course of action is. @chenghuzi @jasmainak it seems like this will be necessary for loading old param files? Unless we enforce that you need to delete drive elements that contain invalid values.

[jasmainak]

sounds like the logic of legacy_mode that you implemented here should actually go inside _extract_drive_specs_from_hnn_params ? So it will return only the non-legacy drives?

[chenghuzi]

sounds like the logic of legacy_mode that you implemented here should actually go inside _extract_drive_specs_from_hnn_params ? So it will return only the non-legacy drives?

Could we have a more structured way of creating networks?

[rythorpe]

What do you propose? At some point we need to cut out all the param file bloat (e.g., consolidate params.py and params_default.py), but the work just hasn't been done yet primarily because of our dependence on legacy_mode.

[ntolley]

Ok I've moved all of the logic into _extract_drive_specs_from_hnn_params so hopefully that's the last of it!

[ntolley]

Just remembered I need to double check how badly the beta example is impacted by this before this PR is set to merge (it's fairly sensitive to the random seed)

[ntolley]

Just checked the beta example locally and it isn't impacted! I'll keep an eye on lingering CI's but I can see the legacy mode cord being cut very soon :smile:

[rythorpe]

Also, looks like something is now off with the seed values that causes test_parallel_backend to fail. I'm guess that a negative seed value gets read in from the .param file which then doesn't get the proper gid-boost that it used to from the ghost poisson drives?

[jasmainak]

This is huge @ntolley !! Don't forget to document in whats_new.rst !!

[rythorpe]

Do you want help with this @ntolley in order to be ready for tomorrow?

[ntolley]

If you have the time to check out the seed problem that'd be great! I'm literally troubleshooting my MPI build right now

[jasmainak]

I updated the PR description to close #526 !

[codecov-commenter]

Codecov Report

Merging #619 (0557bcc) into master (d5d1a03) will decrease coverage by 0.15%. The diff coverage is 100.00%.

:exclamation: Current head 0557bcc differs from pull request most recent head 07f6574. Consider uploading reports for the commit 07f6574 to get more accurate results

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #619      +/-   ##
==========================================
- Coverage   92.19%   92.04%   -0.15%     
==========================================
  Files          22       22              
  Lines        4229     4238       +9     
==========================================
+ Hits         3899     3901       +2     
- Misses        330      337       +7     

Impacted Files	Coverage Δ
hnn_core/drives.py	98.54% <ø> (+0.66%)	:arrow_up:
hnn_core/gui/gui.py	95.02% <ø> (-1.43%)	:arrow_down:
hnn_core/network.py	93.39% <100.00%> (+0.04%)	:arrow_up:
hnn_core/network_models.py	100.00% <100.00%> (ø)
hnn_core/params.py	91.89% <100.00%> (+0.25%)	:arrow_up:
hnn_core/viz.py	89.23% <100.00%> (+0.04%)	:arrow_up:

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[jasmainak]

Thanks @ntolley @rythorpe ! 🥳