Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the usability of PointNeighbors.jl by adding a new Tutorials section to the documentation (generated from Literate.jl sources) and by adding a constructor guard against integer search_radius types that would lead to incorrect/failed distance computations.
Changes:
- Add four new worked tutorials (basic usage, n-body, periodicity, GPU usage) and a Tutorials index page in the docs.
- Integrate Literate.jl generation into
docs/make.jland add required docs build dependencies. - Add an
Integer-typedsearch_radiusvalidation guard to multiple neighborhood search constructors.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/nhs_trivial.jl |
Reject integer search_radius in TrivialNeighborhoodSearch constructor to avoid integer distance conversions. |
src/nhs_precomputed.jl |
Reject integer search_radius in PrecomputedNeighborhoodSearch constructor for the same reason. |
src/nhs_grid.jl |
Clarify n_points docstring wording; reject integer search_radius in GridNeighborhoodSearch constructor. |
docs/src/tutorial.md |
Adds Tutorials landing page with links to each tutorial. |
docs/Project.toml |
Adds Literate, plus tutorial-imported deps (Adapt, KernelAbstractions) to the docs environment. |
docs/make.jl |
Runs Literate conversion into docs/src/tutorials and adds Tutorials to the Documenter page structure. |
docs/literate/src/tut_basic_usage.jl |
New basic fixed-radius neighbor search tutorial source. |
docs/literate/src/tut_n_body.jl |
New n-body cutoff-radius example tutorial source. |
docs/literate/src/tut_periodicity.jl |
New periodic domain tutorial source using PeriodicBox. |
docs/literate/src/tut_gpu_usage.jl |
New GPU usage tutorial source (KernelAbstractions/Adapt-based workflow). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…orhoodSearch.jl into ef/tutorials
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #153 +/- ##
==========================================
- Coverage 86.06% 86.00% -0.06%
==========================================
Files 15 15
Lines 739 743 +4
==========================================
+ Hits 636 639 +3
- Misses 103 104 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/run-gpu-tests |
This has been on my to-do list for a long time. PointNeighbors.jl should be easy to use, but just from the docstrings, it is very difficult to figure out how it works.
This PR
search_radiusis anInteger. This error can happen easily and was previously throwing a cryptic "trying to convert float to int" exception. I don't want to add a hard dispatch toAbstractFloatbecause theoretically, this might work with more special custom data types. Just not with integers.ParallelUpdatethe default strategy for theFullGridCellListwithDynamicVectorOfVectorsbackend. This is a change that I wanted to do for a long time now.ParallelUpdateis faster in most applications, especially on GPUs. Furthermore, it supports initialization on GPUs, whereas the previous default required updating the NHS on the CPU before adapting it to the GPU.Find the rendered docs here: https://trixi-framework.github.io/PointNeighbors.jl/previews/PR153/
Closes #124.