changeset 1752:c98d9c528a22 feature/grids/curvilinear

Review: Added review comments to Grids.jl and mapped_grid.jl
author Vidar Stiernström <vidar.stiernstrom@gmail.com>
date Fri, 13 Sep 2024 10:27:17 -0700
parents e4353d5e8fc3
children 672897f64d58
files src/Grids/Grids.jl src/Grids/mapped_grid.jl
diffstat 2 files changed, 10 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/src/Grids/Grids.jl	Wed Sep 11 15:43:17 2024 +0200
+++ b/src/Grids/Grids.jl	Fri Sep 13 10:27:17 2024 -0700
@@ -1,4 +1,5 @@
 # TODO: Double check that the interfaces for indexing and iterating are fully implemented and tested for all grids.
+# Review: Address this todo?
 module Grids
 
 using Diffinitive.LazyTensors
--- a/src/Grids/mapped_grid.jl	Wed Sep 11 15:43:17 2024 +0200
+++ b/src/Grids/mapped_grid.jl	Fri Sep 13 10:27:17 2024 -0700
@@ -38,6 +38,8 @@
     return same_logicalgrid && same_coordinates && same_jacobian
 end
 
+# Review: rename function logicalgrid to logical_grid
+# for consistency with mapped_grid. 
 """
     logicalgrid(g::MappedGrid)
 
@@ -134,6 +136,13 @@
 #       This would make it well defined also for n-dim grids embedded in higher dimensions.
 # TBD: Is there a better name? metric_determinant?
 # TBD: Is the best option to delete it?
+# Review: I don't think we should delete it. Users building their own 
+#         curvilinear operators will need the functionality. Also the
+#         determinant of the jacobian (and not its square root) is required
+#         for quadratures on mapped grids right? For that reason I think we should
+#         keep the function as is. We could provide a function for the square root
+#         as well if we think it would be helpfull. Regarding naming, perhaps
+#         metric_determinant is better?
 
 """
     metric_tensor(g::MappedGrid)