Change Model::setStateIsNonNegative logic #1648
Conversation
Previously `model.setStateIsNonNegative(model.getStateIsNonNegative())` failed if conservation laws were enabled. That feels unintuitive. Now it works.
Codecov Report
@@ Coverage Diff @@
## develop #1648 +/- ##
===========================================
+ Coverage 78.81% 78.87% +0.05%
===========================================
Files 69 69
Lines 10717 10717
===========================================
+ Hits 8447 8453 +6
+ Misses 2270 2264 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/model.cpp
Outdated
| @@ -609,7 +609,11 @@ std::vector<bool> const &Model::getStateIsNonNegative() const { | |||
| } | |||
|
|
|||
| void Model::setStateIsNonNegative(std::vector<bool> const &nonNegative) { | |||
| if (nx_solver != nx_rdata) { | |||
| auto any_state_non_negative = std::any_of(state_is_non_negative_.begin(), | |||
There was a problem hiding this comment.
I am not sure what's implemented here, shouldn't this be applied to nonNegative? Not implementing this was primarily a result of lazyness. If we specify what kind of indices are passed here (solver or rdata x), we could also support setting non_negative states, just need to map between the two.
There was a problem hiding this comment.
True. Thanks.
I'll open a separate issue for that. #1649
There was a problem hiding this comment.
If we specify what kind of indices are passed here (solver or rdata x), we could also support setting non_negative states, just need to map between the two.
With x_solver indices we wouldn't have to check whether it actually is a solver state. However, I'd guess that most users wouldn't want to care about x_solver at all, and therefore, x_rdata would be more appropriate. Or we just do it ID-based. That's probably the most convenient. What would be your preference?
There was a problem hiding this comment.
If we specify what kind of indices are passed here (solver or rdata x), we could also support setting non_negative states, just need to map between the two.
With x_solver indices we wouldn't have to check whether it actually is a solver state. However, I'd guess that most users wouldn't want to care about x_solver at all, and therefore, x_rdata would be more appropriate. Or we just do it ID-based. That's probably the most convenient. What would be your preference?
I agree, I would expect that users want to pass x_rdata. For id based specification we should probably add another function as changing this function would be API breaking.
|
Kudos, SonarCloud Quality Gate passed! |
Previously
model.setStateIsNonNegative(model.getStateIsNonNegative())failed if conservation laws were enabled. That feels unintuitive.
Now it works.