search

nasa / GSAP

Generic Software Architecture for Prognostics (GSAP)
Other
20 stars 6 forks source link

Fix Async Moving Average Load Est Bug #149

Closed teubert closed 2 years ago

teubert commented 2 years ago

This PR fixes a bug where, when operating asynchronously, the MovingAverageLoadEstimator and any other load-based LoadEstimator does not work correctly. This happens because the add_load method is never called.

The PR fixes the bug by adding a new listener, which listens for the input_vector and adds it to the load estimator. This listener is automatically created by the ModelBasedAsyncPrognoserBuilder

Lumgineer commented 2 years ago

(I was requested in conversation with Chris to take a look at this PR)

My knowledge of GSAP's inner workings is foggy at best, but these changes look like they would fix a potential issue with the asynchronous prognoser that we were running into in the past. To confirm, this issue doesn't affect the simple prognoser at all, only the asynchronous version?

Otherwise, it looks alright to me, but given I can't independently test this, I'd definitely wait on a more comprehensive review.

teubert commented 2 years ago

(I was requested in conversation with Chris to take a look at this PR)

My knowledge of GSAP's inner workings is foggy at best, but these changes look like they would fix a potential issue with the asynchronous prognoser that we were running into in the past. To confirm, this issue doesn't affect the simple prognoser at all, only the asynchronous version?

Otherwise, it looks alright to me, but given I can't independently test this, I'd definitely wait on a more comprehensive review.

Hi @Lumsterling,

That's right that this issue should only be present in async. The load is added using the non synchronous prognoser here: https://github.com/nasa/GSAP/blob/4c4e8266fb34ab96d6d0c27419fcaf2bf5972005/src/ModelBasedPrognoser.cpp#L125

Thanks for finding this bug! Chris