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

Allow recursive includes in parameter files #6175

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

gassmoeller
Copy link
Member

This PR addresses issue #6165 and allows to recursively include parameter files on several levels of input files. See there for a motivation and application cases that are now possible.
I also simplified one benchmark to show what is now possible with this change.

It is worth noting that this PR changes one existing behavior: Currently include statements in input files are not expanded when we copy the input file into the output directory as original.prm, while with this PR they will contain the content of the included files. While this is a change, I actually think the new form is better, because it makes the original.prm more independent from the included file (so they continue to work exactly as they were when the output was generated, even if the base file changes).

@MFraters I think you were interested in this feature ;-)

Copy link
Member

@tjhei tjhei left a comment

Choose a reason for hiding this comment

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

Very nice! Thanks for finally fixing the problem of having to specify dimension directly.

`Some parameter` and `Some other parameter`, then the final file will use the value of
`Some parameter` from `file_b.prm`, but the value of `Some other parameter` from `file_a.prm`.

Also note, that the include statement can include the file path as relative or absolute path,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Also note, that the include statement can include the file path as relative or absolute path,
Also note, that the include statement can include the file path as a relative or absolute path,

source/main.cc Outdated
// by making sure included files are unique, because we may have multiple
// files including the same file in a non-circular way. So we just limit
// the number of included files to a reasonable number.
Assert(n_included_files < 15,
Copy link
Member

Choose a reason for hiding this comment

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

should this be an AssertThrow?

@@ -1,10 +1,93 @@
# test that output/original.prm is written correctly

# based on no_flow.prm:
include $ASPECT_SOURCE_DIR/tests/no_flow.prm
Copy link
Member

Choose a reason for hiding this comment

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

Just an idea for the future: we could automatically add a comment in the original.prm that shows the include statement (or something like "# including content of :")

@gassmoeller gassmoeller merged commit 27bc577 into geodynamics:main Dec 9, 2024
8 checks passed
@gassmoeller gassmoeller deleted the recursive_include branch December 9, 2024 09:19
@MFraters
Copy link
Member

Sorry I didn't get around to look at it earlier, but yes, I think this is cool :)

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.

3 participants