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

Merge in jaycedowell:bifrost/wideband-imager, take 2 #255

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

jaycedowell
Copy link
Collaborator

This PR add the bifrost.orville.Orville gridder/imager to the main Bifrost repo.

@jaycedowell
Copy link
Collaborator Author

jaycedowell commented Jan 8, 2025

There are two things that I haven't resolved with this commit.

The first one is that I haven't figure out is how bifrost.orville and bifrost.romein fit together. Orville is effectively a sub-set of Romein with a few key changes:

  • kernels - Romein expects 4 or 5-D kernels (baselines/pols/size/size) vs the symmetric 1-D kernels used by Orville
  • gridding coordinates - Romein expects integers, Orville floats
  • weights - Orville has an additional "weights" input
  • gridding planes - Romein grids everything onto a single grid, while Orville can use multiple grids for w-projection

There might be other differences but I think those are the key ones.

The second is whether or not any of the additional computational steps in bifrost.orville.Orville.execute() can be moved into orville.cu. Current the .cu file does the gridding and then the Python module handles the summation across gridding planes, image weighthing, the inverse Fourier transform, and the image plane correction for the gridding kernel. Those all end up being calls back into the Bifrost library so moving things into the .cu would save on Python ➡️ C++/CUDA ➡️ Python transitions. I had attempted to do this in an early version but I ended up with problems with bfFft. Maybe that should be revisited.

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.

1 participant