Fix runtime errors in test scripts and utility functions#1
Open
chrishwiggins wants to merge 1 commit intoFrancoisSimon:mainfrom
Open
Fix runtime errors in test scripts and utility functions#1chrishwiggins wants to merge 1 commit intoFrancoisSimon:mainfrom
chrishwiggins wants to merge 1 commit intoFrancoisSimon:mainfrom
Conversation
…data.py - Pass missing pixel_dims arg to emit_photons in generate_movie - Unpack all 4 return values from read_table in test_loading_data.py - Use correct keyword nb_dims (not nb_dimensions) in build_model calls - Add missing initial_fractions argument to build_model calls - Add missing nb_dims and model type column in test scripts - Remove broken tf.math.sigmoid(axis=1) call in model_to_DataFrame - Remove hardcoded model_results[6] debug line in get_number_of_states - Fix double-multiplication of log_likelihood by nb_tracks - Use model.get_weights() instead of get_model_params() in Model_finder Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks
Owner
|
Thank you for the comments. I fixed the issues. |
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
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.
Summary
Hi Francois - congratulations on the ExaTrack paper and release! Really nice work combining the ExTrack and aTrack approaches into a unified framework.
While reading through the code to understand the implementation, I noticed a handful of issues that would cause crashes at runtime. These look like they accumulated naturally as the API in
exatrack.pyevolved (adding the model type column, renaming keyword arguments, changing return signatures) - the kind of thing that's easy to miss when the main fitting scripts are working fine and the test scripts haven't been run in a while.I've tried to keep each fix as small and obvious as possible, changing only what's needed to make the calls match the current signatures.
Changes
exatrack.py(core library)generate_movie/emit_photonsargument mismatch -emit_photonsexpects apixel_dimsparameter, but the call site ingenerate_moviewasn't passing it. Added the missing argument.model_to_DataFrame- invalidtf.math.sigmoidcall - Two lines computedTr_shapeandTr_ratebut were never used, and theTr_rateline calledtf.math.sigmoid(..., axis=1)which isn't a valid signature. Since the transition rates and shapes are already processed in theparamsdict above (softmax and exp already applied), these lines appear to be leftover from an earlier version. Removed them.get_number_of_states- hardcodedmodel_results[6]reference - Linemodel_results[6]['model'].summary()was placed before the results dict is populated and uses a hardcoded key that assumes exactly 6 states. This looks like a leftover debug line. Removed it.get_number_of_states- double log-likelihood scaling - The log-likelihood is already multiplied bynb_trackswhen first computed. A later loop (for k in range(1,7)) multiplied it again, which would corrupt the AIC/BIC values used for model selection. The loop also assumed exactly 6 models via the hardcoded range. Removed the redundant multiplication.Model_finder-get_model_params()returns a dict, not a tuple - The code tried to unpack 5 variables fromget_model_params(model), but that function returns a single dictionary. Changed to usemodel.get_weights(), which matches the pattern used successfully earlier in the same function.test_model.pyMissing 5th column (model type indicator) in
params- Theconstraint_functionreadsparams[4]to determine confined vs. directed motion, but params only had 4 columns. Added the model type column.Missing
initial_fractionsargument -build_modelrequiresinitial_fractionsas a positional argument, but it wasn't being passed. Added it with sensible defaults (equal fractions, low mislinking probability).Wrong keyword
nb_dimensions-build_modelacceptsnb_dims, notnb_dimensions. Fixed the keyword name.test_loading_data.pyUnpacking 3 values from
read_tablewhich returns 4 -read_tablereturns(tracks, frames, track_IDs, opt_metrics)but the script only unpacked 3 values. Addedtrack_ID_listto the unpacking.Undefined variables
nb_independent_varsandnb_dimensions- Addednb_dims = 2and used it consistently.Same
initial_fractionsand model type column fixes as test_model.py.Test plan
python -m py_compilepasses for all three files (confirmed locally)test_model.pyend-to-end with simulated datatest_loading_data.pywith sample CSV dataModel_finderandget_number_of_statesdon't crash on their first callGenerated with Claude Code