-
Notifications
You must be signed in to change notification settings - Fork 68
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
Equipotential plotting #3626
Equipotential plotting #3626
Conversation
…Principia into equipotential-plotting
|
||
struct Equipotentials { | ||
Equipotential<Barycentric, Navigation>::Lines lines; | ||
Parameters parameters; |
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.
It feels a bit odd to return the parameters.
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, but because of the asynchronicity we need them to know whether we can use the result.
if (plotter_idle_) { | ||
plotter_idle_ = false; | ||
plotter_ = MakeStoppableThread( | ||
[this, parameters]() { PlotEquipotentials(parameters).IgnoreError(); }); |
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.
We generally don't write bodies on a single line, so line breaks after {
and before }
.
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.
We do for λs.
Principia/base/recurring_thread_body.hpp
Line 29 in 3fcf46c
[this]() { absl::Status const status = RepeatedlyRunAction(); }); |
&t](Position<Navigation> const& position) { | ||
auto const acceleration = reference_frame.GeometricAcceleration( | ||
t, {position, Velocity<Navigation>{}}); | ||
// Note the sign. |
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 noted it, but maybe explain why it's there.
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.
Done, and renamed the λ.
int __cdecl principia__EquipotentialCount(Plugin* const plugin) { | ||
journal::Method<journal::EquipotentialCount> m({plugin}); | ||
CHECK_NOTNULL(plugin); | ||
plugin->geometric_potential_plotter().RefreshEquipotentials(); |
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.
Why do we refresh if we are not in a rotating pulsating frame? That seems wasteful.
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.
RefreshEquipotentials isn’t RequestEquipotentials (the terminology is the same as in the analyser), the most it does is a swap.
…Principia into equipotential-plotting
#3358.