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

Apply update script to header, source and prm files. #6062

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

gassmoeller
Copy link
Member

This PR applies the update script and also make the scripts a bit safer against accidental shell expansion. Together with #5941 this should also close #6006. If I apply the current update script against ASPECT main, no changes exept for the ones in this PR are made.

@gassmoeller
Copy link
Member Author

Looks like a few tests were affected, because the update script no longer writes the name of the processed file when processing it. What I do not understand is how this bug passed the tester before. It was introduced in #6052 by forgetting to put a : after the number of particles output. I checked that there are many tests that include this output line. It looks like numdiff is ignoring the : sign? I only noticed because when I update the reference results of the failing tests in this PR manually it changed that line in the output. Anyway, it is fixed now, and only a cosmetic change anyway.

@gassmoeller
Copy link
Member Author

@YiminJin you originally reported #6006, could you check if this branch fixes all the problems with the update scripts you observed?

@YiminJin
Copy link
Contributor

@gassmoeller This PR has fixed the bug. I fetched it into my local repository and ran contrib/utilities/update_all_scripts.sh with a new script for ,prm file updating (the one I added for WENO limiter in #5929 ), and everything went well.

Thank you for addressing this issue!

@tjhei tjhei merged commit 6e2d242 into geodynamics:main Oct 11, 2024
8 checks passed
@gassmoeller gassmoeller deleted the apply_update_script branch October 13, 2024 13:28
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.

contrib/utilities/update_scripts/source_files out of date
3 participants