Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fastscapelib (C++) integration #6204

Draft
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

benbovy
Copy link

@benbovy benbovy commented Jan 15, 2025

Related to #5323.

Experimenting with Fastscapelib's flexible grid interface so it is possible to run a Fastscape landscape evolution model based on (any?) deal.ii triangulation used through ASPECT.

cc @MFraters @Minerallo

Before your first pull request:

For all pull requests:

For new features/models or changes of existing features:

  • I have tested my new feature locally to ensure it is correct.
  • I have created a testcase for the new feature/benchmark in the tests/ directory.
  • I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.

@benbovy
Copy link
Author

benbovy commented Jan 22, 2025

I've made some progress and cleaned up a few things:

  • The grid adapter class dealii_grid (in fastscapecc_adapter.h) should now work with most DEAL.II meshes (at least that's the idea). The only constraint is to have a uniform mesh with constant cell area (passed explicitly to the grid adapter constructor).

  • I cleaned up and removed a lot of things inherited from the FastScape plugin (i.e., the one already available in Aspect that uses fastscapelib-fortran). This includes removing plugin parameters and data members for initializing the surface mesh, which we should now get from the geometry model object instead (as implemented for both the box and spherical shell models, although the refinement levels are not correctly defined yet).

    • In particular, there were some geometry model parameters (re-)declared in the plugin's declare_parameters method... Not sure why since those are already declared in Aspect's geometry model component? We can add them back if needed.

Comment on lines 107 to 109
// TODO: reuse Fastscapecc's ``dim`` template parameter here?
// (in theory Fastscapelib C++ may support both 1D and 2D surface domains)
using SurfaceMeshType = Triangulation<2, 3>;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is something available in Aspect, but in theory it would be possible to support a 2-dimensional "disk" geometry model and run this plugin (fastscape erosion) on the 1-dimensional "circle" boundary. I guess that for the general case we could write this instead?

using SurfaceMeshType = Triangulation<dim-1, dim>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is should be <dim-1,dim>, yes

// surface_constraints.distribute(surface_solution);

// Temporary storage for computation
std::vector<std::vector<double>> temporary_variables(dim + 2, std::vector<double>());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: using nested vectors seems unnecessary in my opinion. We could use separate variables instead, which is more readable.

Future proof in case the plugin later supports aspect simulations on
2-dimensional domains (i.e., FastScapecc<2>)

Use SFINAE to select the right implementation of surface mesh
initialization from the input geometry model (still need to check SFINAE
substitution order). Using "if constexpr" would have been better but
Aspect seems to still use C++14 as the lowest required standard.

Also cleaned up header includes and fastscapelib grid adapter.
We don't really need it. We can downcast the geometry model object instead.
std::is_same_v is from C++17 onward.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants