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.

Sign up for GitHub

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

Jump to bottom

BBM in the benchmark case #717

Open
einola opened this issue Oct 21, 2024 · 2 comments
Open

BBM in the benchmark case #717

einola opened this issue Oct 21, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@einola
Copy link
Member

einola commented Oct 21, 2024

We need to be able to run BBM in the benchmark case, like Brodeau et al. (2024), and like we could in the modelling_paper branch.
@einola einola added the bug Something isn't working label Oct 21, 2024
@einola einola mentioned this issue Oct 21, 2024
Merged
12 tasks
@einola
Copy link
Member Author

einola commented Oct 21, 2024 •
edited
Loading

With the current develop, the result for 8, 4, and 2 km setups look like this:

image image image image

@einola
Copy link
Member Author

einola commented Oct 21, 2024

With the fix in PR #718 the fields look essentially the same.

einola added a commit that referenced this issue Oct 21, 2024
@einola
Fix a bug in BBM by adding a +1 to Pmax (#718)
c01279a 
# Fix a bug in BBM by adding a +1 to Pmax

Fixes partially #717

### Task List
- [x] Defined the tests that specify a complete and functioning change
(*It may help to create a [design specification & test
specification](../../../wiki/Specification-Template)*)
- [x] Implemented the source code change that satisfies the tests
- [x] Documented the feature by providing worked example
- [ ] Updated the README or other documentation
- [x] Completed the pre-Request checklist below

---
# Change Description

We need to add a + 1 to the Pmax equation, because we're using the
integrated stress now (so everything's multiplied with h). The correct
equation is

P_{max} =h [ P_0 h^f e^{-C(1-A)} ]  = P_0 h^{f+1} e^{-C(1-A),

which differs from equation (8) of Ólason et al. (2022) by factor h (or
a +1).

---
# Test Description

The benchmark test case now gives more reasonable results, but more work
is required. See issue #717.

---
# Documentation Impact

N/A

---
# Other Details

N/A

---
### Pre-Request Checklist

- [x] The requirements of this pull request are fully captured in an
issue or design specification and are linked and summarised in the
description of this PR
- [x] No new warnings are generated
- [x] The documentation has been updated (or an issue has been created
to track the corresponding change)
- [x] Methods and Tests are commented such that they can be understood
without having to obtain additional context
- [x] This PR/Issue is labelled as a bug/feature/enhancement/breaking
change
- [x] File dates have been updated to reflect modification date
- [x] This change conforms to the conventions described in the README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@einola