Skip to content

Switch sand silt parameters#6950

Merged
gassmoeller merged 6 commits into
geodynamics:mainfrom
anne-glerum:switch_sand_silt_parameters
May 12, 2026
Merged

Switch sand silt parameters#6950
gassmoeller merged 6 commits into
geodynamics:mainfrom
anne-glerum:switch_sand_silt_parameters

Conversation

@anne-glerum
Copy link
Copy Markdown
Contributor

@anne-glerum anne-glerum commented Apr 29, 2026

In line with the updated documentation of FastScape (https://fastscape.org/fastscapelib-fortran/#_fastscape_set_marine_parameters, fastscape-lem/fastscapelib-fortran#54), this PR switches the order of sand and silt parameters when setting the marine parameters in FastScape.

It also renames one parameter to be consistent with the FastScape documentation. This is not backwards compatible.

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.

Copy link
Copy Markdown
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

Ok, we should certainly use the correct order. But just for clarification: Does this mean we were using the wrong order before? Or is this something that changed between Fastscape versions? In this case can we detect the fastscape version we use and adopt the correct order for each? If there are different ways to call this function in different versions we should be either compatible with both, or not allow to use the version not compatible with our call.

@anne-glerum
Copy link
Copy Markdown
Contributor Author

Yes, fastscape.cc had the wrong order before.

FastScape itself didn't change, only its documentation. It documented the wrong order, and therefore the ASPECT FastScape interface also used the wrong order.

At least in our section, no one but me has been using two distinct diffusion coefficents for sand and silt. When they are the same, the order doesn't matter.

Shall I add a change log?

@gassmoeller
Copy link
Copy Markdown
Member

Yes, please add a changelog that the order was wrong before. Just so that users now they should use the new version.

@gassmoeller
Copy link
Copy Markdown
Member

Oh and could you rebase the PR as well? The failing tester should be fixed on the main branch, but I would like to see the confirmation.

@anne-glerum anne-glerum force-pushed the switch_sand_silt_parameters branch from ebbf9e0 to 274ec06 Compare May 5, 2026 08:01
@anne-glerum
Copy link
Copy Markdown
Contributor Author

/rebuild

1 similar comment
@gassmoeller
Copy link
Copy Markdown
Member

/rebuild

@gassmoeller gassmoeller merged commit b898d9c into geodynamics:main May 12, 2026
12 of 14 checks passed
@anne-glerum anne-glerum deleted the switch_sand_silt_parameters branch May 13, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants