REF apply default colors in a few places, drop some comments#2
Merged
Chetank99 merged 2 commits intoMay 28, 2026
Merged
Conversation
Owner
|
Thank you soo much @asoplata !!! |
Chetank99
pushed a commit
that referenced
this pull request
Jun 1, 2026
* rebased Optimization tab addition onto vis refresh This was initially started with some help from Claude AI to form the skeleton, but that only did like the first 15% of the work. Lots of work to follow. temporarily remove CI via comments much progress, next access loaded data it actually works???????????? add proper loading of uploaded data, styling ref: merge constraints into opt_drive_widget data work: abstract widget gen, hboxes, begin syns Syns appear to be a special case since they're packaged as dictionaries to be sent to `net.add_<drive>`. Current implementation works for HBoxes and GUI as a whole, BUT will fail at sim time, since I accidentally changed the `weights_ampa`, etc. dicts. These dicts need to be reverted, and instead of them being changed, we should just add new dicts with the same structure, like `weights_ampa_opt_min`, etc. where each celltype (key) inside the dict gets its own checkbox bool, min, and max. Then, inside our closure'd `set_params`, add some separate logic that processes the OPT params in order to fully form the weights before the drives are created. woohoo got synapses working ref: cleanup and error checking in main opt run [no ci] uncomment workflows to avoid weird errors [no ci] add GUI selection of max_psd obj fun [no ci] add remaining changes for gui psd opt run [no ci] add observe drive vars<-> opt vars also some comment cleaning [no ci] add ghosting and propn ctrl for max_psd [no ci] add Poisson widgets but cant sim yet also minor ref [no ci] ref constraint builder, use poisson now [no ci] adapt and deploy rhythmic drive [no ci] save work, tonic opt widget broken [no ci] finish getting tonic to work [no ci] Clean up. https://www.youtube.com/watch?v=Uc44K5ypQxE [no ci] ref observe func & ghost ALL drive vars [no ci] ref/doc: renames and Opt function docs [no ci] add delete-equivalence b/w Drives/Opt tabs [no ci] reload GUI drive params after succ opt run [no ci] many small things and try checkbox-state [no ci] large number of changes 1. Include and use an "n_trials" in the dipole_rmse case, including have the new Opt widget dlink with the default Run ntrials widget value 2. Successfully apply prior state of Optimization checkboxes, etc. whenever `HNNGUI.opt_widget_values` is re-done. (This way, users don't have to re-click (or remember!) what they were optimizing against). All drive types now support this prior state usage. 3. Make `_build_constraints` a top-level function for use in the prior task 2 (above). Also, fix a "bug" where it is now also used for non-synaptic constraints in drives that support them (see task 6 below). 4. Update all Opt tab variable widgets that show the same information as corresponding Drives tab widgets, such that they are dual-linked to each other. This way, if someone wants to change the "base" value of a certain drive variable, they don't have to go to the Drives tab to do so. Moreover, changes to that variable in the Opt tab will propagate to the same variable in the Drive tab. Moreover, retain the inter-widget control between certain special variable widgets (like "No. Drive Cells" and "Cell-Specific"). 5. Remove the ability to optimize against `numspikes`/"No. Spikes". This is needed because our Optimization functions don't account for constraint values that must be integers, as opposed to floats. This also means that currently, we cannot optimize against any variables that are integers, including also "No. Drive Cells" and Seeds. I've also culled the relevant code from `_create_opt_widgets_for_var`. 6. Give many remaining drive variables the ability to be optimized against, especially the many time and rate variables of Poisson and Rhythmic drives. 7. Fix minor bug where final output plot was only using the first possible name of the final (optimized) output simulation name (sim + "_optimized"), instead of the proper new name of the final output sim. [no ci] remove debug/comments, and incr n_cores ruff format and revert n_cores change minor changes to the only current gui opt test reduce max opt iter [no ci] link opt tstop and regular [no ci] doc: add freq band units I'm so tired. ref: change opt min/max to percentages This was partially done using Claude AI, but everything looks good to me. ref: track GUI state of opt target param widgets This was mostly done using Claude-AI. ref: opt psd band2 checkbox updates proportions ref: add Optimization History reporting This was mostly done with Claude-AI. fix: target opt data RMSE widget resets correctly This fixes a small bug where, before, if there was a pre-existing simulation data set present (such as from an earlier regular run or `maximize_psd` optimization run), then the `dipole_rmse` "Target Data" widget would incorrectly use the first value of its list as the target data, instead of the selected value. This turned out to be because, in the changed code, setting the options of the widget using ``` self._external_data_widget.options = all_sim_names ``` actually changes the value to the first entry in the list, even if the prior value is a legitimate element of the new `options`. This commit adds a tiny amount of code to workaround this, so that the old value is retained, and the user-selected option for Target Data is used, even in the event of there being a pre-existing simulation that is indexed before any loaded data. review: incr opt initial constr to +/- 50% gui: add inter-linked dt and n_jobs w's to Opt tab gui opt: remove timestamp, show all obj_ vals [no ci] ugh ruff doc: better docstring for vizman.update_external ref: replace double-`dlink` with single `link` doc: document initial Opt tab sections ref: apply @ntolley renames [no ci] doc: apply various @ntolley comment/doc changes [MRG] bugfix: Update args to fix GUI viz tests (jonescompneurolab#1217) * [MRG] bugfix: Update args to fix GUI viz tests This is a tiny, trivial bugfix that fixes the currently failing tests on `master`. `master` currently fails `test_gui.py::test_gui_visualization` due to an error in the `"network"` plot_type case of the main `for` loop. The failing testing code is here: ```python setup_gui = <hnn_core.gui.gui.HNNGUI object at 0x10a36f050> def test_gui_visualization(setup_gui): """Tests updating a figure creates plots with data.""" gui = setup_gui # Spectrogram needs longer time for wavelet analysis gui.widget_tstop.value = 500 gui.run_button.click() gui._simulate_viz_action("switch_fig_template", "[Blank] single figure") gui._simulate_viz_action("add_fig") figid = 2 figname = f"Figure {figid}" axname = "ax0" for viz_type in _plot_types: gui._simulate_viz_action( "edit_figure", figname, axname, "default", viz_type, {}, "clear" ) # Check that extra axes have been successfully removed assert len(gui.viz_manager.figs[figid].axes) == 1 # Check if data on the axes has been successfully cleared assert not gui.viz_manager.figs[figid].axes[0].has_data() gui._simulate_viz_action( "edit_figure", figname, axname, "default", viz_type, {}, "plot" ) # Check if data is plotted on the axes > assert gui.viz_manager.figs[figid].axes[0].has_data() E AssertionError: assert False E + where False = has_data() E + where has_data = <Axes: label='Spike histogram'>.has_data hnn_core/tests/test_gui.py:718: AssertionError ``` while the actual problem is in the provided Traceback here: ```python Traceback (most recent call last): File "/opt/anaconda3/envs/hc11/lib/python3.11/site-packages/ipywidgets/widgets/widget.py", line 191, in __call__ local_value = callback(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/austinsoplata/rep/brn/hnn-core/hnn_core/gui/_viz_manager.py", line 577, in _plot_on_axes dpls_processed = _update_ax( ^^^^^^^^^^^ File "/Users/austinsoplata/rep/brn/hnn-core/hnn_core/gui/_viz_manager.py", line 441, in _update_ax img_arr = np.reshape( ^^^^^^^^^^^ TypeError: reshape() got an unexpected keyword argument 'newshape' ``` Specifically, the root cause is that `_viz_manager.py::_update_ax()`, in the `plot_type == "network"` case, makes a call to `np.reshape`. The named parameter in that call is named `newshape`, however, in Numpy 2.4.0, this named arg was changed to `shape`, which you can see in the release notes here: https://numpy.org/doc/stable/release/2.4.0-notes.html#removed-newshape-parameter-from-numpy-reshape Renaming the named arg allows all tests to pass once more. Note that after merge, this change will need to be merged/rebased into any existing PRs in order for them to pass tests. * fix: remove named arg entirely for older versions doc: single line comment ref: add cleaner all-drive-delete [no ci] ref: move Opt history table to genOpt [no ci] doc: improve "prior" opt widget docs [no ci] wow Copilot is SCARY good at predicting useful comment text... ref: combine _build and _extract_prior_constraints ref: move `set_params` helper functions outside This moves the former local functions `name_check`, `use_nonsyn_params_if_exists`, and `create_parametrized_syn_dicts_if_exist` outside of their original location, which was inside the `set_params` closure inside `_generate_constraints_and_func`. This also prepends an underscore to their function names, and adds full docstrings for these three functions. ref: remove time from Opt-able params This removes time parameters such as `tstart`, `tstart_std`, `tstop`, and `t0` from the list of parameters that can be optimized against in the GUI for Poisson, Rhythmic, and Tonic drives (but NOT `mu`/`sigma` for Evoked drives). ref: change Opt constraints to absolute pct This changes the way Optimization GUI tab constraint minima and maxima to use "absolute" percentages instead of "relative", as discussed in jonescompneurolab#1190 (comment) and jonescompneurolab#1190 (comment) At a 1-on-1 meeting with Nick and I, we decided to switch to the "absolute" way. Now, min/max constraint percentages are never negative, and "100%" is more easily communicated to be the exact current value of the parameter. [WIP] Ref/deduplicate opt drive tab code (#2) * ref: merge drive & opt Evoked gen funcs This was mostly done with Claude * ref: merge drive & opt Poisson gen funcs This was mostly done with Claude * ref: merge drive & opt Rhythmic gen funcs This was mostly done with Claude * ref: merge drive & opt Tonic gen funcs This was mostly done with Claude * ref: merge drive/opt syn weight gen funcs This merges _get_drive_weight_widgets and _create_synaptic_widgets_for_opt into _create_synaptic_widgets This was mostly done with Claude. * ref: merge general drive/opt drive gen funcs This merges add_drive_widget and add_opt_drive_widget into a new function named _create_widgets_for_drive. This was mostly done with Claude. * ref: a few brief renames b/w drive/opt tabs [no ci] * fix: resolve test incompat b/w gui and j093x3 For some weird reason, there's a new bug appearing after the auto-refactors that comes up because `jones2009_3x3_drives.json` has a different expectation of the L2_pyramidal rate_constant from the network that gets created for the GUI. I need to investigate this, but either: 1. the auto-refactors introduced a bug, or 2. this elevated a previously unknown bug or inconsistency with how some of our test (or base) networks have their Poisson drive configured * ref: simple renames This renames the following functions: - `HNNGUI.add_drive_widget` -> `HNNGUI.add_drive_tab_drive_widget` - `HNNGUI.add_opt_drive_widget` -> `HNNGUI.add_opt_tab_drive_widget` Inside the functions ``` HNNGUI.add_drive_tab_drive_widget HNNGUI._create_widgets_for_drive HNNGUI._create_widgets_for_evoked HNNGUI._create_widgets_for_poisson HNNGUI._create_widgets_for_rhythmic HNNGUI._create_widgets_for_tonic ``` This renames the following variables: - `drive` to `new_drive_widgets` - `drive_box` to `new_drive_box` This is so that inside the functions, the main output variables are not confused with input drive data, or the drive name, or anything else. * ref: rename opt style/layout vars (1 of 2ish) This is the first of a a couple of commits cleaning up the layout/style variable passing throughout the drive creation stuff in the GUI. This renames all Optimization-tab related layout and style variables to be prefixed with `opt_tab_`. This also creates new variables `opt_tab_var_layout` and `opt_tab_var_style` starting at `add_opt_tab_drive_widget`. These new variables are passed all the way down the call chain and used by `add_opt_tab_drive_widget` to define the style/layout of all "base" variable widgets (via using `kwargs` in various `_create_widgets_for_XXX` functions). This allows for easier future refactoring, since BEFORE, the Optimization tab was still using the un-prefixed `layout` and `style` arguments throughout the call chain, BUT in a way that is different than how they're used in the Drive tab. Honestly I'm not explaining it well, but just trust me, it'll make way more sense what I'm doing after another commit or two. * ref: some simple renaming of the diff kwargs * ref: rename drive style/layout var This turns the formerly required positional arguments for the (Drive tab's) layout and style variables into optional variables that have different names. This allows us to make our kwargs handling easier to follow and document. This also re-orders some of the many drives arguments. * ref: rename for_opt to req'd "choose" argument This renames `for_opt` to `choose_tab_drive_or_opt`, makes it a required argument, and forces it to use a value of either `drive` or `opt`. This also updates all the control flow to use this new approach. With this, this series of refactors is now complete. * ref: always cap tstop for rhy & pois drives This was probably originally introduced by Claude because the Drive tab version did the right thing by using `tstop_widget.value`, while my original Opt tab implementation used 1e6. * doc: add some comments * ref: distribute drive widget collection * ref: regroup synapse style args in drives * doc: tiny comment changes [no ci] * bugfix: opt pois lambdas were not passed before * ref: alphabetize main drive funcs * ref: split used data vs default_data in drive func * ref: refactor syn widg creation, more readable * ref: unnecessary if * ref: consolidate numspikes, tiny fmt [no ci] * ref: clean poisson and rhythmic [no ci] * ref: add note about tonic GUI, add times title * fix: save opt history button [no ci] * test: add joblib to gui opt test [no ci] * fix typo from merge conflict [no ci] * style: revert button style after merge conflict [no ci] * ref: remove mutable defaults [no ci] * test: all drives in gui opt test [no ci] * fix: opt tonic constraint carryover * test: beeeg update to main gui opt test * test: add post-opt-run constraint value check [no ci] * test: add opt inp fail tests, decr time, fix tonic test * ref: 1 of N adjusting GUI opt code & style to visual refresh * ref: 2 of 2 adapting Opt tab styling/flow to usual * maint: in-place replacement by Nick's CMA branch * maint: re-add back opt history table * feat: add `dipole_corr` support to GUI Opt * [WIP] debug: add first CMA try to GUI opt (jonescompneurolab#3) * debug: add first CMA try to GUI opt * maint: remove issue-metrics Github Action (jonescompneurolab#1279) As of the most recent U24 Steering Committee Meeting, we have effectively replaced measuring the metrics of our issues and PRs with Dylan's solution available here jonescompneurolab/hnn-tracking#1 This also allows us to query the information as-needed. We therefore decided in the HNN Code Review meeting on 2026-04-02 to remove usage of the `issue-metrics` Action. * DEBUG: attempt no backend with GUI CMA [no ci] @ntolley take a look at this. This is not a working solution yet - this takes all the code under the "with backend" in the main opt run function, and makes a separate copy that does NOT use a backend, exclusively for the CMA solver. Then, the only different for these blocks of code is solely that in the CMA case, "n_jobs" is being passed to obj_fun_kwargs. However, I'm getting some very confusing errors about serialization comin from Loky, so there could be some major problems. (For example, even though I do not believe we're passing anything non-serializable, something could be interfering with CMA's use of BatchSimulate's use of Joblib and maybe trying to serialize part of the GUI). * DEBUG: remove widgets from closure and hardcode popsize @ntolley Claude Opus found what I think was the existing issue, in that in the `set_params` closure, there were a few ipywidgets being used, but those were interfering with the serialization/pickling by Loky. That definitely makes sense that that could have been the problem. I've included Claude's changes here to strip the widgets from the closure for testing by us tomorrow, though I haven't looked at the changes very closely. Also, I have hardcoded the popsize to be lower since the testing was taking a while to run. Of course, popsize should maybe be a user-configurable value from a widget, but we can do that after we get everything working. This appears to pass `test_gui.py::test_gui_run_optimization` on my machine. We should discuss more tomorrow. * ref/debug: functionalize the main execution for QOL * Fix _hammfilt input validation and improve test coverage (jonescompneurolab#1266) * Fix _hammfilt input validation and improve test coverage * Address review: remove redundant validation and update tests * align and update tests * ref: minor changes * ref: replace util asserts with error-checks [no ci] * test: update test for better util error [no ci] * doc: improve docs and errors in utils [no ci] * test: 1 of 2 improving smooth_waveform tests [no ci] * test: more and better smoothing tests * doc: fix typos and clean comments --------- Co-authored-by: Austin E. Soplata <me@asoplata.com> * [MRG]: Re-enable Codecov (jonescompneurolab#1272) * Re-enable Codecov uploads via codecov-action v5 * CI: Remove Codecov token usage After following the instructions for https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token I used admin privileges to "install the Codecov Github app" for the HNN-Core repository. Then, I went to codecov.io, logged into the organization's account (after logging into my personal github account), went to Settings, went to Global Upload Token, and changed it to "Not required". In theory, everything should work now. This commit will also act as a test to see if it does. * debug: bump to investigate failing check * doc: add contribution [no ci] --------- Co-authored-by: Austin E. Soplata <me@asoplata.com> * [MRG] Optimization enhancements: CMA-ES algorithm and correlation loss function (jonescompneurolab#1221) * start opt drive function * Move CMA and corr functions to optimization files * read dt, allow n_trials, update external_drives * clean up optimization * add cma dependency and start tests * ruff formatting * fix tests * fix tests * add batch sim detection to all objective functions * reformatting * fix sigma * undo verbose checks * change default njobs * cleanup rebase * cleanup rebase * refactor is_batch check * clean up verbose and refactor preprocessing * reformat psd calculation * ruff cleanup * add warning * ruff * docstring * remove unecessary test * add docstrings about initial params for CMA * refactor dipole sampling * fix args * ruff * fix init imports * return weights * ruff * tests ofr sigma0 * typos * get rid of initial params error * remove obsolete test * ruff * remove commented out test * rename to anticorr * improve coverage * more tests * more tests --------- Co-authored-by: katduecker <katharina.duecker@gmail.com> * [MRG] Expose option to control random seed in CMA optimization (jonescompneurolab#1286) * update cma code with seed parameter * fix tests * fix tests * set_dt * numpy all in tests * add documentation * clean up tests * fix ruff complaints by proper opt exec passing * ref: doc and improve usage of "snapshot" drives "Snapshot drives" refers to the use of the new `_snapshot_drive_widgets`, which is used to extract all the values that we need for `_generate_constraints_and_func`, but without extracting any Widget objects, which are not picklable. * ref: simplify gui opt drive-snapshot code * test: decr test run time * feat: 1 of 2 for adding CMA solver widgets * feat 2/2 using CMA solver options in gui opt --------- Co-authored-by: Satvik Saluja <satviksaluja2507@gmail.com> Co-authored-by: vaishnavi baghel <vshnvibaghel45@gmail.com> Co-authored-by: Nicholas Tolley <55253912+ntolley@users.noreply.github.com> Co-authored-by: katduecker <katharina.duecker@gmail.com> * ruff lol * important fix and remove some not all debug * ref: remove unnec flag, remove all debugs * fix: avoid mindelay sim resolution * debug: attempt to decrease sim time (check popsize output) * fix: tonic ampl bug in gui opt * test: greatly shorten tests of gui opt * debug: attempt to shorten tests * fix: revert amplitude dict swap (needs investigation) * ruff * fix: use Claude to debug Tonic bias issues [no ci] * test: attempt 4 to minimize time to test gui opt * DOC comment cleanup [no ci] * REV many small changes recommended by Nick * REV better handling of pre-opt sim and params * ruff * REV apply remaining small Nick comments * DOC spelling and nick comment * DOC add comments for future ref in GUI Opt [no ci] * TEST apply new nick comments * DOC add more notes on future refactors [no ci] * DOC better comments for default drive params [no ci] --------- Co-authored-by: Nick Tolley <nic.m.tolley@gmail.com> Co-authored-by: Satvik Saluja <satviksaluja2507@gmail.com> Co-authored-by: vaishnavi baghel <vshnvibaghel45@gmail.com> Co-authored-by: Nicholas Tolley <55253912+ntolley@users.noreply.github.com> Co-authored-by: katduecker <katharina.duecker@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.