Skip to content

NF - Pft accepts any directiongetter#12

Open
gabknight wants to merge 1 commit intogabknight:masterfrom
JesusMda:pft-accepts-any-directiongetter
Open

NF - Pft accepts any directiongetter#12
gabknight wants to merge 1 commit intogabknight:masterfrom
JesusMda:pft-accepts-any-directiongetter

Conversation

@gabknight
Copy link
Owner

@gabknight gabknight commented Jul 7, 2025

Description by Korbit AI

What change is being made?

Refactor the particle filter tracking (PFT) process to remove the use of the particle_dirs and directions arrays, allowing the algorithm to accept any DirectionGetter implementation for direction determination.

Why are these changes being made?

The removal of the directions and particle_dirs arrays streamlines PFT by leveraging the direction calculations provided directly by the DirectionGetter, reducing redundancy and simplifying the overall code structure. This enhances flexibility, allowing for various DirectionGetter strategies to be used without being tied to predefined direction arrays, promoting modularity and code reusability within the dipy tracking framework.

Is this description stale? Ask me to generate a new description by commenting /korbit-generate-pr-description

@JesusMda JesusMda force-pushed the pft-accepts-any-directiongetter branch from 3209d35 to 6abf3f3 Compare July 7, 2025 13:13
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Inefficient Point Operations ▹ view
Functionality Infinite Loop in ENDPOINT Handling ▹ view
Performance Redundant PVE Calculations ▹ view
Files scanned
File Path Reviewed
dipy/tracking/local_tracking.py
dipy/tracking/localtrack.pyx

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

Repository owner deleted a comment from korbit-ai bot Jul 24, 2025
Repository owner deleted a comment from korbit-ai bot Jul 24, 2025
Repository owner deleted a comment from korbit-ai bot Jul 24, 2025
Copy link
Owner Author

@gabknight gabknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JesusMda . Looks good to me. Can you address my minor comments. Can you check the PFT tests and add one that test various DG for PFT?

Then do a rebase on dipy/master, and do a PR on the main repo?

i += i_sub-1
copy_point(&streamline[i-1,0], point)
# update max_wm_pve
for j in range(i_sub):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a check here, and only do the for loop if

max_wm_pve < min_wm_pve_before_stopping.

i.e. once we reached the wm, we don't need to update this variable.

# update the current point with the PFT results
copy_point(&streamline[i-1, 0], point)
copy_point(&directions[i-1, 0], &direction[0])
# copy_point(&directions[i-1, 0], &direction[0])
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable direction is not used, no? If this is correct, could you remove the direction variable completely, and rename direction_calc to direction

break
# Update the point and prev_point variables and get the direction of the last step
stream_status_1 = stream_status[0]
if i>1:
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you re-organize the code to start the loop with the call to dg.generate_streamline. This if i>1 and the i_sub = 2 seems unnecessary if you re-order the logic.

Repository owner deleted a comment from korbit-ai bot Jul 25, 2025
Repository owner deleted a comment from korbit-ai bot Oct 2, 2025
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.

2 participants