-
Notifications
You must be signed in to change notification settings - Fork 65
Allow different Fortran source directory #675
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
Conversation
|
I have a couple of questions about this PR:
|
I also have a couple of questions! Regarding your question 1. Would it be too difficult to use/reuse/abuse the existing Regarding your question 2. How is capgen handling the In general, the use case for this new functionality is to be able to add metadata files for code that we do not "own", e.g. when we pull in a physics scheme for an authoritative source as a submodule? |
|
I thought |
I am fairly confident that there is currently no case where there is more than one file with the correct name, and it wouldn't be overly restrictive to make this a requirement? |
I've seen duplicate filenames in a number of projects. DART comes to mind. RRTMGP has two files named mo_gas_optics_rrtmgp_kernels.F90 (and possibly other duplicates, I did not check). |
True, but if I understand this PR and the underlying issue correctly, the restrictions would apply to files that correspond to a CCPP scheme (i.e. a metadata file with a certain prefix) only? In other words, if |
|
I'd vote for a separate (optional) metadata field as Steve implemented here, rather than extending the use of As for the name, I can't think of anything better than |
|
|
peverwhee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one optional thing
| if(DEFINED arg_VERBOSITY) | ||
| string(REPEAT "--verbose" ${arg_VERBOSITY} VERBOSE_PARAMS_SEPERATED) | ||
| separate_arguments(VERBOSE_PARAMS UNIX_COMMAND "${VERBOSE_PARAMS_SEPERATED}") | ||
| string(REPEAT "--verbose " ${arg_VERBOSITY} VERBOSE_PARAMS_SEPARATED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for fixing this!
| dependencies = <dependencies> | ||
| module = <module name> # only needed if module name differs from filename | ||
| source_path = <relative source directory of Fortran source (if different)> | ||
| dynamic_constituent_routine = <routine name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops! I know this is unrelated to your changes, but can you remove this residual dynamic constituents line left by me?
dustinswales
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Thanks for adding this functionality. It will come in handy for rte-rrtmgp!
Add a new metadata table keyword,
source_path, which allows a Fortran source file to be in a different directory from its metadata file.source_pathis a path relative to the metadata file path.User interface changes?: Yes
New optional metadata table keyword,
source_path.Backwards compatibility is maintained by using the metadata file path as the default value for the Fortran source path.
Testing:
test removed: none
unit tests: pass
system tests: pass (capgen test modified to test new feature)
manual testing: NA
Fixes #674
Fixes #676