-
Notifications
You must be signed in to change notification settings - Fork 188
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
Update Inspiral.yaml #6291
Update Inspiral.yaml #6291
Conversation
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.
LGTM, just one small requset for clarification :)
support/Pipelines/Bbh/Inspiral.yaml
Outdated
- Lapse | ||
- ConstraintEnergy |
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.
I'm a bit surprised we write out the constraint energy and lapse. I think these could be computed from the 3 evolved variables. It would be good to have a comment why they are observed instead of computed when needed.
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.
Oh, we don't think we need to output those for the transition to ringdown script to work, they were originally there to see where the constraints were blowing up or getting large. I can take that out
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.
For the pipeline and transition to ringdown, yeah we probably don't need these.
support/Pipelines/Bbh/Inspiral.yaml
Outdated
- Lapse | ||
- ConstraintEnergy |
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.
For the pipeline and transition to ringdown, yeah we probably don't need these.
9f8dad2
to
00d5fa1
Compare
Squashed those changes in @nilsdeppe and @knelli2 :) |
@AlexCarpenter46 Could you rebase on latest develop? There's a conflict now |
06a8526
00d5fa1
to
06a8526
Compare
Rebased :) @knelli2 |
06a8526
to
d794a34
Compare
Proposed changes
Updates the Inspiral.yaml to be what worked well in @geoffrey4444's BBH runs for the paper. This also starts to set up the volume data needed for the transition to ringdown.
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
Do we want to cleanly terminate the run at a certain separation?