Skip to content

Bugfix in subroutine init_atm_case_mtn_wave regarding parallel execution #1257

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

Closed
wants to merge 1 commit into from

Conversation

MHBalsmeier
Copy link

This is a fix for issue #1256

The problem was that the center coordinate of the domain xc was not synchronized among processes which resulted in wrong orography. This is now fixed and the initial state files created in parallel and serially are identical.

@mgduda
Copy link
Contributor

mgduda commented May 12, 2025

@MHBalsmeier Thanks very much for finding this bug and providing a fix. I have just three small requests:

  1. Rather than opening a PR from your develop branch, could you commit the fix to a new branch based on the develop branch and open the PR from that branch?
  2. In the new PR, could you target the hotfix-v8.2.3 branch?
  3. In the commit message, could you include an explanation of the bug and how the changes in the commit fix that bug? We use the output of git log quite often, and so more detail in the commit messages can be valuable.

Thanks again, and if you have any questions, please don't hesitate to follow up!

@MHBalsmeier
Copy link
Author

Thank you, sure, I'll open a new one.

@MHBalsmeier
Copy link
Author

@mgduda "commit the fix to a new branch based on the develop branch and open the PR from that branch": I based my new PR on the current develop branch of this repo, and not on my develop branch ... so now it would merge all the commits in develop into the hotfix branch. It's showing 110 commits. Is that what you intended?
I might have to try a third time, sorry for the chaos.

@mgduda
Copy link
Contributor

mgduda commented May 13, 2025

@MHBalsmeier I think you'll be able to fix up PR #1312 by force-pushing changes to your existing init_atm_cases_bugfix branch. So, there's probably not a need to make a third PR.

@MHBalsmeier
Copy link
Author

@mgduda Thank you, let's see how it looks now.

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.

2 participants