nasa / GSAP

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

Model improvements #26

Closed jason-watkins closed 6 years ago

jason-watkins commented 6 years ago

This commit does three things:

I haven't made any attempt to propogate the contract API throughout the code here, just used it to express a missing constraint in the battery model. Future work will see it used extensively in updates to the Observer and Predictor implementations.

This code was originally written before the incident, and while I have spent a lot of time making sure that it doesn't reintroduce any errors, I would like special attention paid to making sure that nothing has been accidentally reverted to pre-1.1.1 code.

julianvu commented 6 years ago

Jason and I were also concerned about certain parts of the code base after the resolution of "the incident." We are unsure if some pieces are relics from a previous version or if they are meant to be there. In particular, we're unsure about whether the PredictedOutputs were moved to the model from the predictor or the reverse.

teubert commented 6 years ago

The predicted outputs names were moved from the predictor to the PrognosticsModel, so they're in the same place. I did look through the things that I remember being changed in the branch that was lost and I think they are all there. To the best of my knowledge your merger is complete.

Are there any other specific examples that you were unsure of?

teubert commented 6 years ago

I continued to look through it and I did find something: predictor.events key should not be in the predictor anymore. Since the events are now a property of the models, I had removed that key and had the predictor use whatever the model supplies as the events.

teubert commented 6 years ago

Also, I tried building this on my Mac, and it would not build. It seems that the Assert.h file breaks it on my Mac (despite passing the Travis.ci test). @julianvu Does it work on your machine?

It's very strange, It's giving a number of unrelated errors. I added the commits one by one to and found that it breaks on "Fix Test Errors". I added the changes one by one and it seems that even having a file called Assert.h breaks everything on my Mac. Here's all I have in Assert.h:

`#ifndef PCOE_ASSERT_H

define PCOE_ASSERT_H

include "ThreadSafeLog.h"

endif / PCOE_ASSERT_H /`

If I remove the #Include "ThreadSafeLog.h line everything compiles, but as soon as I have any instructions at all in the file it breaks

julianvu commented 6 years ago

GSAP builds on my Mac but only with CLion. Xcode is where the build breaks. It might be because Xcode has its own Assert.h for its unit testing framework.