Fix half plane issues - #94
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Sorry @steven-murray, your pull request is larger than the review limit of 150000 diff characters
|
@nikos-triantafyllou this is finally ready for review -- there were a couple of small bugs on the 21cmSense side which are now fixed in a PR there (this branch adds the specific 21cmSense branch as a dependency... once you check this and give the all clear, I'll pull the trigger on the 21cmSense PR and update |
|
@nikos-triantafyllou it looks like it will fail tests until I merge and release the new 21cmSense, but you should be able to install it with BTW this does not attempt to fix #104 at all -- once this is merged into |
|
View / edit / reply to this conversation on ReviewNB nikos-triantafyllou commented on 2026-06-18T11:55:38Z Line #7. from py21cmsense import Observation, Observatory, convert_half_to_full_uv_plane Maybe do the same for 21cmsense to make it clear where the functions are coming from. Not a priority though, can be another PR. |
|
View / edit / reply to this conversation on ReviewNB nikos-triantafyllou commented on 2026-06-18T11:55:39Z Line #3. lc_with_noise = observe_lightcone( Nice clean up! |
nikos-triantafyllou
left a comment
There was a problem hiding this comment.
Just a small change in the testing file, I assume you wanted to check the imaginary part as well.
| - name: Install Non-Python Dependencies | ||
| run: | | ||
| conda install fftw gsl | ||
| conda install pip fftw gsl numba compilers pkg-config |
There was a problem hiding this comment.
Where do we use numba?
There was a problem hiding this comment.
We don't explicitly but I think one of our deps does and it's easier to install with conda in this case
Co-authored-by: Nikos Triantafyllou <nikolaos.triantafyllou@sns.it>
This is some work I'm doing to see if I can fix the half-plane vs full-plane issues. Just a draft for now as I'm working on it between this and the 21cmSense PR.