RealOutputs for thermal sensors#385
Conversation
Using RealOutputs for thermal sensors. Removing usage of OrdinaryDiffEqDefaults in tests
jbatteh
changed the title
| @@ -1,4 +1,4 @@ | |||
| using ModelingToolkit, OrdinaryDiffEqDefault, Test | |||
There was a problem hiding this comment.
remove from the test part of the Project toml as well?
|
@baggepinnen does this constitute a breaking change? |
|
Technically yes, any model that previously used this blocks will be broken. The blocks were pretty useless since they had no output connector, so in practice I'm not sure if they were used all that much and thus if this PR will actually break anything |
| @@ -1,4 +1,4 @@ | |||
| using ModelingToolkit, OrdinaryDiffEqDefault, Test | |||
| using ModelingToolkit, Test | |||
There was a problem hiding this comment.
what's the point of the removal if OrdinaryDiffEq is directly used in others?
There was a problem hiding this comment.
I definitely didn't understand some complexities here. When I instantiated based on Project.toml, I didn't get OrdinaryDiffEq in my environment at all. I manually added it to run some of the other tests (though perhaps it wasn't really needed there). But even so, the tests that had this uses annotation with OrdinaryDiffEqDefault were complaining that this package was missing So I just removed in these tests. I am definitely interested in knowing what the best way to approach would be.
There was a problem hiding this comment.
I use TestEnv.jl yoninstantiate the test environment, which is generally different from the package environment, notably, tests often require additional dependencies.
|
Okay so @baggepinnen you agree with these changes? |
3 participants
#384
Using RealOutputs for thermal sensors. Note that any existing usage of temperature sensors will need to be updated (i.e. referencing of sensor output goes from T -> T.u). Connections to sensor output are now possible. Thermal sensors are now following the same style as those in Rotational with sensor output on connectors.
Removing usage of OrdinaryDiffEqDefaults in tests
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
No new tests were added but existing tests in Thermal package all pass. Note that usage of OrdinaryDiffEqDefaults was removed from a few tests. This package was not included when I instantiated and doesn't seem necessary. Please check if this change is also OK?
Add any other context about the problem here.