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

ncells gets same value for all features in one timestep #246

Closed
JuliaKukulies opened this issue Feb 13, 2023 · 4 comments
Closed

ncells gets same value for all features in one timestep #246

JuliaKukulies opened this issue Feb 13, 2023 · 4 comments
Assignees
Labels
bug Code that is failing or producing the wrong result High Priority This issue needs immediate fixing, and may warrant a hotfix release
Milestone

Comments

@JuliaKukulies
Copy link
Member

JuliaKukulies commented Feb 13, 2023

Thanks to @laurapaccini who posted in #245 that ncells in the feature output from the segmentation do not always correspond to the number of grid cells associated with the feature in the segmentation mask.

Eventually, I found the problem in the segmentation code, where we iterate over the different features in one time step, but assign only the value of ncells of the last feature to all features :

tobac/tobac/segmentation.py

Lines 267 to 275 in 09b176b

# count number of grid cells asoociated to each tracked cell and write that into DataFrame:
values, count = np.unique(segmentation_mask, return_counts=True)
counts = dict(zip(values, count))
ncells = np.zeros(len(features_out))
for i, (index, row) in enumerate(features_out.iterrows()):
if row["feature"] in counts.keys():
ncells = counts[row["feature"]]
features_out["ncells"] = ncells

I double-checked this behavior in our example notebooks, which all show only one unique value of ncells per timestep, although there are multiple features detected in each timestep.

This bug will be relatively easy to fix but should be done asap and preferably with a test making sure that this does not happen again.

Since we are about to release v1.5 maybe it suffices to include this in v1.5? But I leave it to @w-k-jones and @freemansw1 to judge if another hotfix is needed.

@JuliaKukulies JuliaKukulies added bug Code that is failing or producing the wrong result High Priority This issue needs immediate fixing, and may warrant a hotfix release labels Feb 13, 2023
@JuliaKukulies JuliaKukulies added this to the Version 1.5 milestone Feb 13, 2023
@JuliaKukulies JuliaKukulies self-assigned this Feb 14, 2023
@freemansw1
Copy link
Member

I'm inclined to release this as a 1.4.2 hotfix; I'd rather have a 1.4.x version that we know works available as we introduce the variety of new features in 1.5.0.

@w-k-jones
Copy link
Member

I agree, happy to expedite this as a 1.4.2 release prior to 1.5

@JuliaKukulies
Copy link
Member Author

True, that is a good point. I can take care of this

JuliaKukulies added a commit that referenced this issue Feb 18, 2023
Hotfix v1.4.2 to resolve bug with ncells in feature dataframe #246
@JuliaKukulies
Copy link
Member Author

This has been solved with #247 and will be released in v1.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code that is failing or producing the wrong result High Priority This issue needs immediate fixing, and may warrant a hotfix release
Projects
None yet
Development

No branches or pull requests

3 participants