Make TreeMesh type general

[DanielDoehring]

No description provided.

[github-actions]

Review checklist

This checklist is meant to assist creators of PRs (to let them know what reviewers will typically look for) and reviewers (to guide them in a structured review process). Items do not need to be checked explicitly for a PR to be eligible for merging.

Purpose and scope

• The PR has a single goal that is clear from the PR title and/or description.
• All code changes represent a single set of modifications that logically belong together.
• No more than 500 lines of code are changed or there is no obvious way to split the PR into multiple PRs.

Code quality

• The code can be understood easily.
• Newly introduced names for variables etc. are self-descriptive and consistent with existing naming conventions.
• There are no redundancies that can be removed by simple modularization/refactoring.
• There are no leftover debug statements or commented code sections.
• The code adheres to our conventions and style guide, and to the Julia guidelines.

Documentation

• New functions and types are documented with a docstring or top-level comment.
• Relevant publications are referenced in docstrings (see example for formatting).
• Inline comments are used to document longer or unusual code sections.
• Comments describe intent ("why?") and not just functionality ("what?").
• If the PR introduces a significant change or new feature, it is documented in NEWS.md with its PR number.

Testing

• The PR passes all tests.
• New or modified lines of code are covered by tests.
• New or modified tests run in less then 10 seconds.

Performance

• There are no type instabilities or memory allocations in performance-critical parts.
• If the PR intent is to improve performance, before/after time measurements are posted in the PR.

Verification

• The correctness of the code was verified using appropriate tests.
• If new equations/methods are added, a convergence test has been run and the results
  are posted in the PR.

Created with ❤️ by the Trixi.jl community.

[DanielDoehring]

Maybe good point in time to combine this with #1194

[ranocha]

Maybe good point in time to combine this with #1194

👍

[codecov]

Codecov Report

Attention: Patch coverage is 93.54839% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.36%. Comparing base (dcf1b58) to head (7582f1b).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/meshes/abstract_tree.jl	66.67%	2 Missing ⚠️
src/meshes/tree_mesh.jl	88.89%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2129      +/-   ##
==========================================
- Coverage   96.36%   96.36%   -0.00%     
==========================================
  Files         480      480              
  Lines       38028    38042      +14     
==========================================
+ Hits        36645    36657      +12     
- Misses       1383     1385       +2     

Flag	Coverage Δ
unittests	96.36% <93.55%> (-<0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[huiyuxie]

Also in parallel_tree and serial_tree there are some Float64 NaNs - for example see

Trixi.jl/src/meshes/parallel_tree.jl

Lines 190 to 204 in 8743712

	function invalidate!(t::ParallelTree, first::Int, last::Int)
	@assert first > 0
	@assert last <= t.capacity + 1
	# Integer values are set to smallest negative value, floating point values to NaN
	t.parent_ids[first:last] .= typemin(Int)
	t.child_ids[:, first:last] .= typemin(Int)
	t.neighbor_ids[:, first:last] .= typemin(Int)
	t.levels[first:last] .= typemin(Int)
	t.coordinates[:, first:last] .= NaN
	t.original_cell_ids[first:last] .= typemin(Int)
	t.mpi_ranks[first:last] .= typemin(Int)
	return nothing
	end

Basically NaN of type Float64 will not cause type instability since they will make everything NaN in numerical operations but I'm worried about something like

t.coordinates[:, first:last] .= one(typeof(t.coordinates[1, 1])

So it would be better to make NaN also change with the input type how do you think about it @ranocha?

[huiyuxie]

👍👍👍

[ranocha]

Also in parallel_tree and serial_tree there are some Float64 NaNs - for example see

Trixi.jl/src/meshes/parallel_tree.jl

Lines 190 to 204 in 8743712

	function invalidate!(t::ParallelTree, first::Int, last::Int)
	@assert first > 0
	@assert last <= t.capacity + 1
	# Integer values are set to smallest negative value, floating point values to NaN
	t.parent_ids[first:last] .= typemin(Int)
	t.child_ids[:, first:last] .= typemin(Int)
	t.neighbor_ids[:, first:last] .= typemin(Int)
	t.levels[first:last] .= typemin(Int)
	t.coordinates[:, first:last] .= NaN
	t.original_cell_ids[first:last] .= typemin(Int)
	t.mpi_ranks[first:last] .= typemin(Int)
	return nothing
	end

Basically NaN of type Float64 will not cause type instability since they will make everything NaN in numerical operations but I'm worried about something like

t.coordinates[:, first:last] .= one(typeof(t.coordinates[1, 1])

So it would be better to make NaN also change with the input type

That should happen right now?

julia> foo!(x) = (x .= NaN; x)

julia> foo!(zeros(Float64, 10))
10-element Vector{Float64}:
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN

julia> foo!(zeros(Float32, 10))
10-element Vector{Float32}:
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN

julia> foo!(zeros(Float16, 10))
10-element Vector{Float16}:
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 NaN
 
julia> foo!(zeros(Float16, 10))[1]
NaN16

julia> foo!(zeros(Float32, 10))[1]
NaN32

julia> foo!(zeros(Float64, 10))[1]
NaN

[ranocha]

@huiyuxie If you like, please review this PR first and request my review when you're done.

[huiyuxie]

please review this PR first and request my review when you're done.

👌and what is your foo! function definition @ranocha ?

[huiyuxie]

Or we could also keep one interface for each - SerialTree{NDIMS, RealT} and SerialTree{1, RealT} (and similarly, ParallelTree{NDIMS, RealT} and ParallelTree{1, RealT} ) and change original SerialTree{NDIMS} to SerialTree{NDIMS, RealT} and SerialTree{1} to SerialTree{1, RealT} (and also for parallel tree) in precompile.

Which way would you prefer @DanielDoehring @ranocha ?

[ranocha]

please review this PR first and request my review when you're done.

👌and what is your foo! function definition @ranocha ?

Sorry, fixed above

[ranocha]

Thanks. Looks quite good already 👍

[ranocha]

Thanks