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

Solve bug in SpinRanges when indexing a phantom with a BitVector #520

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

pvillacorta
Copy link
Collaborator

@pvillacorta pvillacorta commented Dec 9, 2024

This worked:
obj.motion = Translate(5e-4, 6e-4, 7e-4, TimeRange(0.0, 1.0), SpinRange(obj.x .> 0))

But this did not work, and it was not tested:
obj = obj[obj.x .> 0]

When executing this second line, the resulting phantom had lost its motion. Two additional method definitions for the intersect_idx function inside SpinSpan.jl can solve this.

I also use this PR to solve a bug in get_spin_coords(m::Motion). I initialized the displacement buffers as follows:
ux, uy, uz = x .+ 0*t, y .+ 0*t, z .+ 0*t
And this is not correct, since it unnecessarily adds the initial positions to the displacements.

This is how it is solved:
ux, uy, uz = x .* (0*t), y .* (0*t), z .* (0*t)

@pvillacorta pvillacorta marked this pull request as ready for review December 9, 2024 22:58
@cncastillo
Copy link
Member

I believe you can also use similar(a, M, N).

I will wait until the tests pass!

@pvillacorta
Copy link
Collaborator Author

@cncastillo It should be ready now

@cncastillo
Copy link
Member

I still see some AMDGPU tests failing

@pvillacorta
Copy link
Collaborator Author

I think it's a random error that has nothing to do with the code.

@cncastillo
Copy link
Member

You are right, I re-run that job, to see if it is fixed.

@cncastillo cncastillo merged commit 6d4deb8 into master Dec 10, 2024
8 of 12 checks passed
@cncastillo cncastillo deleted the easy-motion branch December 10, 2024 21:25
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