Adding 2-D tracer transport in thetis #122

thangel · 2018-01-26T13:47:45Z

adding the option to solve advection diffusion equation in 2D

…model :Q#

* don't do `if logical_variable==True:` * overshoot and conservation tests weren't actually run

New TracerMassConservation2DCallback wasn't actually used.

So it actually fails if we accidentily use the 3d conservation callback.

tkarna

This looks great! Only minor comments

tkarna · 2018-02-05T02:30:28Z

thetis/solver2d.py

@@ -212,6 +216,8 @@ def create_function_spaces(self):
            raise Exception('Unsupported finite element family {:}'.format(self.options.element_family))
        self.function_spaces.V_2d = MixedFunctionSpace([self.function_spaces.U_2d, self.function_spaces.H_2d])

+        self.function_spaces.Q_2d = FunctionSpace(self.mesh2d, 'DG', 1, name='Q_2d')


The tracer space is called H in the 3D model, we could use the same symbol for consistency?

Yeah, the reason I suggested a different symbol is that I interpreted H as the pressure/elevation space - which in 3D has to be same as the tracer space. For depth averaged tracers, I think we don't get strict conservation anyway, so perhaps we could be more flexible. Also strictly speaking I think the requirement is for the tracer space to be a subspace of the pressure space, so in the case of P1DG-P2 you could use P1 tracers.

I see, yes we already use H_2d for the elevation space.

tkarna · 2018-02-05T02:31:32Z

thetis/solver2d.py

+            self.fields.tracer_2d = Function(self.function_spaces.Q_2d, name='tracer_2d')
+            self.eq_tracer = tracer_eq_2d.TracerEquation2D(self.function_spaces.Q_2d, bathymetry=self.fields.bathymetry_2d,
+                                                           use_lax_friedrichs=self.options.use_lax_friedrichs_tracer)
+            self.eq_tracer.bnd_functions = self.bnd_functions['tracer']


Assigning the bnd functions to the equation here. Is this being used? We also set the bnd functions in the coupled time integrator class.

tkarna · 2018-02-05T02:38:23Z

test/tracerEq/test_h-diffusion_mes_2d.py

+"""
+Testing 2D horizontal diffusion of tracers against analytical solution.
+
+Tuomas Karna 2015-12-14, Athanasios Angeloudis 16/01/2018


Should we keep these author tags in the source code? To me it's unclear if they're useful. Currently there are some from 2015 that mention me, but no others...

tkarna · 2018-02-05T02:59:58Z

test/tracerEq/test_h-diffusion_mes_2d.py

+    options.horizontal_viscosity_scale = Constant(horizontal_diffusivity)
+    options.update(model_options)
+    if hasattr(options.timestepper_options, 'use_automatic_timestep'):
+        options.timestepper_options.use_automatic_timestep = True


Related to setting the horizontal_viscosity_scale: it seems that solver2d.set_time_step does not take viscosity scale into account when computing the time step, therefore use_automatic_timestep is not really meaninful, especially here where we are testing diffusion only. See #79 . Doesn't have to be implemented in this PR.

tkarna · 2018-02-05T03:15:24Z

test/tracerEq/test_h-diffusion_mes_2d.py

+
+    # project analytical solultion on high order mesh
+    t_const.assign(t)
+    tracer_ana_ho.project(tracer_expr)


Same comment on high order projection.

tkarna · 2018-02-05T03:23:00Z

test/tracerEq/test_h-advection_mes_2d.py

+
+    # project analytical solultion on high order mesh
+    t_const.assign(t)
+    tracer_ana_ho.project(ana_tracer_expr)


I think projection to high-order function space is no longer necessary if tracer_expr is pure UFL expression; errornorm should just work. This is a remnant of times when it didn't.

tkarna · 2018-02-05T03:25:11Z

thetis/field_defs.py

@@ -64,6 +64,12 @@
    'unit': 'm s-1',
    'filename': 'Velocity2d',
 }
+field_metadata['tracer_2d'] = {
+    'name': 'Tracer',


Depth averaged?

tkarna · 2018-02-05T03:27:19Z

thetis/coupled_timeintegrator_2d.py

+
+    def _create_tracer_integrator(self):
+        """
+        Create time integrator for salinity equation


tracer equation?

tkarna · 2018-02-05T03:31:07Z

thetis/tracer_eq_2d.py

+
+class TracerEquation2D(Equation):
+    """
+    3D tracer advection-diffusion equation :eq:`tracer_eq` in conservative form


tkarna · 2018-02-05T03:31:48Z

thetis/tracer_eq_2d.py

+        """
+        :arg function_space: :class:`FunctionSpace` where the solution belongs
+        :kwarg bathymetry: bathymetry of the domain
+        :type bathymetry: 3D :class:`Function` or :class:`Constant`


tkarna · 2018-02-05T03:34:06Z

One missing bit is the documentation. At least 2D model formulation should be updated. Would also be nice to have a 2D demo with tracers. Can be in another PR.

tkarna

I think the high order spaces could be removed altogether from the tests. Also remove thetis/.solver2d.py.swp file.

tkarna · 2018-02-13T04:18:58Z

test/tracerEq/test_h-advection_mes_2d.py

+            next_export_t += solverobj.options.simulation_export_time
+            iexport += 1
+
+    # project analytical solultion on high order mesh


typo: solution

tkarna · 2018-02-13T04:19:09Z

test/tracerEq/test_h-diffusion_mes_2d.py

+            next_export_t += solverobj.options.simulation_export_time
+            iexport += 1
+
+    # project analytical solultion on high order mesh


tkarna · 2018-02-13T04:26:13Z

test/tracerEq/test_h-advection_mes_2d.py

+
+    p1dg_ho = FunctionSpace(solverobj.mesh2d, 'DG', options.polynomial_degree + 2,
+                            vfamily='DG', vdegree=options.polynomial_degree + 2)
+    tracer_ana_ho = Function(p1dg_ho, name='tracer analytical')


I guess my point on the high order projection was that p1dg_ho and tracer_ana_ho are no longer needed. You can just compute the error as errornorm(tracer_ana_ufl_expr, tracer_sol). I think /test/pressure_grad has some examples (degree_rise option is also obsolete).

Oh now I see, yes I just realised that these do not make a difference! Updating it now

tkarna · 2018-02-21T14:21:42Z

Thanks for the updates, looks good to me!

Athanasios Angeloudis and others added 30 commits January 26, 2018 13:34

added tracer_eq_2d.py file to be adapted for tracer transport in 2-D …

232113c

…model :Q#

commented sections of 3-D version

f2edf25

more commented sections of 3-D version

c1ea280

more commented sections of 3-D version

d5e8470

Deleted sections of 3-D version and adapted some args - needs checking

434a1b6

Deleted sections of 3-D version and adapted some args - needs checking

209254e

applying minor changes on tracer_eq_2d.py

a6c5c05

Recent changes for tracer equation 2d

578e435

Changes made by Than and Stephan for the 2-D tracer transport module

b2328cd

Consolidating tracer_2d in solver_2d

c4bbeb1

Adding tracer 2d options

5933638

Recent changes to make tracer transport work

07e9318

latest changes to horizontal advection and testing

ea6bf7d

Edits to tracer module

b559326

recent changes while testing tracer timestepper

97bceb8

Adding tracer conservation and overshoot in 2d

cbb1886

test_consistency_2d test

6279391

updating test_consistency test

6600f75

update test_consistency_2d.py

6511fe0

adding test on tracer advection in 2d

3bd7d7e

adding diffusion test- please check

6442703

Changes and addition of diffusion test

f189c55

Addinf mms case for tracer .. testing.

0394694

Addition of theta option and mms test

c37cdf1

edits

fc6e13a

Some fixes in test:

0829d68

* don't do `if logical_variable==True:` * overshoot and conservation tests weren't actually run

Fix test descriptions

169845d

Minor cleaning of test.

efd9117

Make sure test actually runs (in pytest).

01a477f

Commented out code.

19d63a1

stephankramer and others added 8 commits January 26, 2018 13:37

Not needed.

a7613fb

Name classname consistently.

306146b

Remove spurious _2d in check_tracer_[conservation/overshoot]_2d option.

8dffa28

Let's ensure we're actually using the right conservation callback!

dc09dc3

New TracerMassConservation2DCallback wasn't actually used.

Make sure this test actually properly tests 2d conservation

ddc21cf

So it actually fails if we accidentily use the 3d conservation callback.

make lint corrections

9121357

Remove some spurious brackets

6772068

Fix after rebase.

fcae93c

thangel requested a review from tkarna January 26, 2018 13:47

tkarna requested changes Feb 5, 2018

View reviewed changes

thangel added 4 commits February 8, 2018 09:44

Revisions requested

3060e67

Revisions requested

4bc9081

lint changes

f154180

Revisions for pull request

5f32a15

tkarna reviewed Feb 13, 2018

View reviewed changes

thangel added 2 commits February 13, 2018 09:53

Corrections to tests

222a45a

updating tracer name

68208a0

tkarna approved these changes Feb 21, 2018

View reviewed changes

thangel merged commit 0e9f313 into master Feb 22, 2018

thangel deleted the than_tracer branch February 26, 2018 09:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding 2-D tracer transport in thetis #122

Adding 2-D tracer transport in thetis #122

thangel commented Jan 26, 2018

tkarna left a comment

tkarna Feb 5, 2018

stephankramer Feb 7, 2018

tkarna Feb 8, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna Feb 5, 2018

tkarna commented Feb 5, 2018

tkarna left a comment

tkarna Feb 13, 2018

tkarna Feb 13, 2018

tkarna Feb 13, 2018

thangel Feb 13, 2018

tkarna commented Feb 21, 2018

Adding 2-D tracer transport in thetis #122

Adding 2-D tracer transport in thetis #122

Conversation

thangel commented Jan 26, 2018

tkarna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkarna commented Feb 5, 2018

tkarna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkarna commented Feb 21, 2018