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

Fix: Better first_point fix #96

Merged
merged 59 commits into from
Jan 25, 2024
Merged

Fix: Better first_point fix #96

merged 59 commits into from
Jan 25, 2024

Conversation

arnaudon
Copy link
Contributor

@arnaudon arnaudon commented Dec 1, 2023

I fixed a mismatch of growth here: #75
but I now realise that this was adding 2 points at each growth iteration, not just when we bifurcate. This solution should be less disruptive.

More details on how this fixes asymmetry issue. I added a test that makes this morphology:
Screenshot 2023-12-06 at 15 09 10

and we have the resulting synthetic apical without this, or #75 on the left, and with on the right:
Screenshot 2023-12-06 at 15 06 59

the left morphology has no obliques on the left branch, because the right one had the firs bif, then was 1 micron ahead of the left branch, hence has always more probability to pick up the next bif, and this become exponentially the case.
Now we add a point also in the other branches, so they are all at the same path distance from soma when growing, ensuring equal probability for both apical stubtrees to bifurcate to obliques.

@arnaudon arnaudon self-assigned this Dec 1, 2023
neurots/generate/tree.py Outdated Show resolved Hide resolved
neurots/generate/tree.py Outdated Show resolved Hide resolved
arnaudon and others added 3 commits December 1, 2023 17:07
Co-authored-by: Adrien Berchet <adrien.berchet@epfl.ch>
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

Merging #96 (7a29457) into main (9d74824) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #96   +/-   ##
=======================================
  Coverage   97.76%   97.77%           
=======================================
  Files          39       39           
  Lines        2197     2199    +2     
  Branches      381      383    +2     
=======================================
+ Hits         2148     2150    +2     
  Misses         30       30           
  Partials       19       19           
Flag Coverage Δ
pytest 97.77% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
neurots/astrocyte/space_colonization.py 94.32% <100.00%> (ø)
neurots/generate/orientations.py 100.00% <ø> (ø)
neurots/generate/tree.py 100.00% <100.00%> (ø)

@arnaudon arnaudon marked this pull request as ready for review December 6, 2023 08:37
@arnaudon
Copy link
Contributor Author

arnaudon commented Dec 6, 2023

so the change in neurots/astrocyte/space_colonization.py is the minimal thing I could find to stabilise the test between bb5/mac/github ci. I'm happy to have another solution, but I did not find one so far. If I understood well, the small numerical imprecisions due to numpy/scipy on different machines becomes large due to this normalisation of length vector when it is small (In the test, it can be 1e-7, so small deviations of 1e-10 or below get normalised and jump to 1e-3, which is at the edge of the accuracy of the test, and break them in this PR). It may also be beneficial to regularise this term in general, to prevent numerical instabilities, but I'm not sure on the repercusion of this rounding, and if there is a better way to do it in a least disruptive way

@arnaudon arnaudon merged commit 3f1b2a8 into main Jan 25, 2024
9 checks passed
@arnaudon arnaudon deleted the fix_first_point branch January 25, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants