Update DERA output and minor fixes #302
Conversation
|
Performance Results
ResidualModel and MassMatrixModel performance results should be compared between versions and not between models due to the execution order of the tests |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #302 +/- ##
==========================================
+ Coverage 90.16% 90.80% +0.63%
==========================================
Files 63 63
Lines 7637 7634 -3
==========================================
+ Hits 6886 6932 +46
+ Misses 751 702 -49
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
This change looks good. The DERA model is currently not fully complete --- the low and high voltage / frequency tripping and reconnection logic is not yet implemented. We had some discussion a while back and I think the conclusion was the capability did not yet exist to properly implement these timer based functionalities without major new features in PSID. Should there be a warning with this model? Should it be documented?
The warning that we currently have when constructing the Simulation is sufficient for now IMO |
| @@ -571,6 +571,7 @@ function initialize_dynamic_device!( | |||
| end | |||
|
|
|||
| set_P_ref(dynamic_wrapper, Pref) | |||
| PSY.set_P_ref!(dynamic_device, Pref) | |||
There was a problem hiding this comment.
we need to either fix all the setters or not overload this one.
There was a problem hiding this comment.
This is part of a major issue related with output results that we would need to properly fix once we do a restructure of output results.
I need to add this line because I need Pref to compute the output current, but I don't have access to the wrapper in the simulation outputs, that is the one that has the information about Pref. So I overload this info in the initialization.
This creates another problem though, because if we change Pref from a callback, only the information stored in the wrapper will change (and hence correctly in the simulation), but the output result will not change. We would need to correct this, since is similar as the issue #269
The change include an error for DynamicInjection cases.