make legacy_mode=False in the GUI

[jasmainak]

It should be done here:

https://github.com/jonescompneurolab/hnn-core/blob/d1f57b2fede9d74ea04845c512fa7411c7503777/hnn_core/gui/gui.py#L900

cc @ntolley @rythorpe @chenghuzi

[rythorpe]

What's the rationale here? This will cause the simulations generated by the old GUI and the current GUI to diverge. If we're willing to make this jump, we might as well get rid of legacy mode altogether.

[jasmainak]

Currently I see this with the default parameters:

I find this very confusing to explain to users ... there are only 3 drives in the default parameters but all these extra drives turn up.

Also, this starts to become a circular argument ... when we try to remove the legacy mode in hnn-core, then the question is to make it match with the GUI. That's why I propose removing it from the GUI, then next step make legacy_mode=False the default. We can keep legacy_mode=True a possibility for now if people are in the middle of research projects. But eventually remove the parameter in a release or two.

[chenghuzi]

If we set legacy_mode to false, the hnn-core will complain that with default.json we have:

This is due to https://github.com/jonescompneurolab/hnn-core/blob/8bf415dd1b0afa7abe83c510eb64f48bd0743e64/hnn_core/param/default.json#L152

and the tstop is 170

[jasmainak]

You should do a separate PR to fix this. It's really unfortunate that our default legacy_mode=True which means all these checks are skipped. @ntolley @rythorpe can you come to the meeting tomorrow so we can discuss and come to a consensus?

[jasmainak]

I'm marking this as a blocker for 0.3 ... no new release until we have fixed this

[jasmainak]

@chenghuzi @rythorpe @ntolley I think the right solution is:

• make legacy_mode=False as default
• Set legacy_mode=True only in tests
• Raise deprecation warning if legacy_mode=True
• Modify the default.json to remove drives that are after tstop. This necessarily means we will break compatibility with old GUI and this is on purpose ... we do not want to indefinitely carry the ghosts of the old GUI. Those wishing to reproduce the old GUI should use the old GUI.
• Have a default_old_gui.json that is used to test correctness by checking against old GUI results.

Let's modify this blueprint during the meeting depending on the discussion. We also need to decide who will take the lead on this.

[rythorpe]

@chenghuzi @rythorpe @ntolley I think the right solution is:

• make legacy_mode=False as default
• Set legacy_mode=True only in tests
• Raise deprecation warning if legacy_mode=True
• Modify the default.json to remove drives that are after tstop. This necessarily means we will break compatibility with old GUI and this is on purpose ... we do not want to indefinitely carry the ghosts of the old GUI. Those wishing to reproduce the old GUI should use the old GUI.
• Have a default_old_gui.json that is used to test correctness by checking against old GUI results.

....or just get rid of it altogether and modify our tests accordingly.

[jasmainak]

To reiterate what we discussed in the meeting with @rythorpe first step here is to consolidate the code to read param files in one location

[jasmainak]

@chenghuzi @rythorpe any further thoughts here? I think this is important to consider before the 0.3 release.

[rythorpe]

I agree that this is definitely important, although implementing this might not be a trivial PR.

[rythorpe]

I'm imagining that we modify this at the level of the Network class and within each of our template network models - is that what you were thinking @jasmainak?

[jasmainak]

I guess so ... it feels like this is a code sprint kind of PR

[chenghuzi]

Could we do it now or we leave it to the moment when everything else is ready? It looks straightforward though

[jasmainak]

Is this something we could prioritize for the code sprint? Could the work be divided somehow? The new GUI still shows 5 or 6 drives which is very confusing for the user I feel ...