-
Notifications
You must be signed in to change notification settings - Fork 192
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
Fix bug in PreprocessCceWorldtube #6459
Fix bug in PreprocessCceWorldtube #6459
Conversation
docs/Tutorials/CCE.md
Outdated
@@ -292,6 +292,9 @@ Here are some notes about the different options in the YAML input file: | |||
simulation that provided the boundary dataset. | |||
- `FixSpecNormalization` should always be `False` unless you are using a | |||
particualy old version of SpEC | |||
- `DescendingM` should always be `False`. This option is to maintain |
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.
This sentence could be confusing because DescendingM
depends on how people provide their metric modal data. It can be True
. I guess you are referring to the output Bondi modal data, which always have ascending m.
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.
Yes technically it can be true for metric modal data, but I think we want to encourage people to always have their data in ascending m order (unless it's from spec). Hence the strong language that it should always be False.
docs/Tutorials/CCE.md
Outdated
@@ -292,6 +292,9 @@ Here are some notes about the different options in the YAML input file: | |||
simulation that provided the boundary dataset. | |||
- `FixSpecNormalization` should always be `False` unless you are using a | |||
particualy old version of SpEC | |||
- `DescendingM` should always be `False`. This option is to maintain | |||
compatibility with older versions of SpEC. Our |
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.
Current SpEC still dumps metric modal data with descending m (even though this option has been disabled in a default run). I would be more explicit here:
This option is to support metric modal data from SpEC.
This executable can run properly. |
The option FixSpecNormalization was incorrectly used to mean "file is spec format" (aka descending m order). Adds a new option to the Preprocess executable and input file to account for this
b771d8d
to
4bc9457
Compare
Looks good. |
Proposed changes
Fixes #6450.
Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments