-
Notifications
You must be signed in to change notification settings - Fork 191
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
Add single BH control system #6247
Add single BH control system #6247
Conversation
bb49248
to
0fb8665
Compare
|
||
namespace control_system::ControlErrors { | ||
template struct Translation<1_st>; | ||
template struct Translation<2_st>; |
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.
These don't do anything useful because the class is entirely defined in the hpp.
support/Pipelines/Bbh/Ringdown.yaml
Outdated
@@ -246,6 +246,22 @@ ControlSystems: | |||
WriteDataToDisk: false | |||
MeasurementsPerUpdate: 4 | |||
Verbosity: Silent | |||
Translation: | |||
IsActive: false |
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.
Did you want to turn this on?
0fb8665
to
8333b04
Compare
Squashed everything in |
8333b04
to
5f44dd3
Compare
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.
clang-tidy timed out for no obvious reason. Rerunning to see if it was some sort of glitch. If it still times out I'm not going to worry about it.
@wthrowe I've seen clang-tidy time out on several PRs lately for what seems like no good reason (not that many files changed). I haven't tried to figure out why though |
Sounds like you fixed it in #6282. Want to rebase to pick that up? |
5f44dd3
to
f08d0da
Compare
I don't think #6282 was the issue as I'm pretty sure the clang-tidy timeouts were happening before I introduced that, but I rebased just in case. |
Actually, looks like your fix broke it more. CI is spewing errors. |
And then reporting itself as passed. |
f08d0da
to
4025f32
Compare
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 like that fixed clang-tidy. One of the errors there is legitimate, but it wasn't introduced in this PR and we should get the fix in quickly, so I'm not going to worry about it.
Proposed changes
Controls object to the origin of the domain.
Upgrade instructions
Must now specify a
Translation:
control system inEvolveGhSingleBlackHole
input files.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