Skip to content
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

Interrupt the orbit analysis if the mission duration changes #2799

Merged
merged 11 commits into from
Nov 28, 2020

Conversation

eggrobin
Copy link
Member

@eggrobin eggrobin commented Nov 27, 2020

In order to do that:

  • refactor the orbit analyser a bit so that its thread can be restarted;
  • fix the operator= of our implementation of jthread;
  • introduce RETURN_IF_STOPPED;
  • pepper it throughout OrbitalElements and related functions;
  • avoid copies of large vectors, wherein we cannot RETURN_IF_STOPPED;
  • in particular, update Status for move semantics (what year is this).

The analyser can lag a little bit when updating the mission duration during the actual analysis (as opposed to the integration), I think the remaining large atomic operations are vector destructions; not much we can easily do about those.

Drive-by changes: take an exclusive lock to swap, not a reader lock, don’t flow twice to the same time.

MeanEquinoctialElements(orbital_elements.osculating_equinoctial_elements_,
orbital_elements.sidereal_period_);
RETURN_IF_ERROR(mean_equinoctial_elements.status());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change the macro to automatically extract the status when give a base::StatusOr.

Sorry, something went wrong.

base/jthread.hpp Outdated
@@ -123,6 +124,13 @@ class this_stoppable_thread {
friend jthread MakeStoppableThread(Function&& f, Args&&... args);
};

#define RETURN_IF_STOPPED \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this should take (optionally) a parameter to let the caller propagate a useful message.

I am also wondering if our macro names should systematically start with PRINCIPIA_.

Sorry, something went wrong.

@pleroy pleroy added the LGTM label Nov 28, 2020
@eggrobin
Copy link
Member Author

retest this please

@eggrobin eggrobin merged commit 932348f into mockingbirdnest:master Nov 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants