-
Notifications
You must be signed in to change notification settings - Fork 169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit error in Modelica.Clocked.Examples.Systems.Utilities.ComponentsThrottleControl.IntakeManifold #4097
Comments
I'll bring up one more, similar, unit error also affecting the In the Line 28 in 7051091
As far as I understand, the only unit of angle that would allow this kind of equation would have been Edit: Fixed typo |
You mean "deg" - not "degC". To me that is one of the problematic cases: someone has fitted a 3rd degree polynomial between angle and angle (?):
Interestingly it could be that Added: For the first someone it seems to be: Prabhakar, R. 1975,“Optimal and Suboptimal Control of Automotive Engine Efficiency and Emissions,” Ph.D. Thesis, Purdue University. West Lafayette, IN |
And more importantly. Why does it say:
|
What is the current thinking/best practices for these types of equations (in the first post) and what does the spec say about it, if anything? It's not yet obvious there there is an issue here. Bear with me here as I explain my thinking and understanding...
I expect the answer to following question should help guide us: for the equation below, should 1.0 be assumed to have units of "1" or units of "" (which could potentially be deduced to have units of [y]/[x])? Furthermore, should there be any difference in assumption between cases 1 and 2 below?
In case 1, And then in case 2, the question i posed earlier naturally arises: should the modelica tool assume 1.0 has units of "" or "1"? If the answer is that the units should be assumed to be "1", then I agree there is an issue here. But if the assumption is that the units are "" and could later be deduced, I see no such issue. At the very least I'd like to see this clearly specified in the modelica spec. |
In the language group, we have worked quite a bit towards standardization in the area of unit checking last year. The topic is bit, but our starting point has been precisely to make it clear that the literal
See:
Please join us in these specification topics if you are interested, and let us try to keep these design discussions in the ModelicaSpecification repository. |
Thanks. I suppose I was aware of the items you mentioned, but they are either "open" or in "draft". Shouldn't we wait until they are closed to declare items like this as an "issue". Or would you characterize the situation as the writing is on the wall, and values like Also, can you clarify your current expectation for case 1: I assume/hope that one is not considered an issue? I think this is relevant for this issue because it might drive what good should look like/how to resolve the issue here. For example, if I have the following equation:
The key is while I would normally declare the units for x1, x2, and y, I'm not going through and declaring the units for the 6 coefficients (leaving them as ""). |
Yes, if you ask me. In addition to all ongoing language design discussions, I believe this principle is also going to be enforced one way or another also in Dymola. Note that Modelica tools already have unit checking features even though there is no formal specification of what can be checked. Still, I'd say it's a good idea to pay attention to (all?!) unit problems reported by tools, as this will increase changes of finding errors, and reduce risk of having to fix these problems later when they become errors according to the standard. |
We're on a slippery slope here, with clear risk of ending up in design discussions in the wrong place. My expectation for 1 is that initial specification of unit checking will leave it undefined, allowing it to at least pass for some time. However, with a slight variation as |
To make it work out unit-wise, I'd recommend creating helper variables with unit
|
I think we need to discuss this first. On one hand it makes sense and we don't want to allow such sloppy unit-mixing everywhere, but:
One possibility would be to have some annotation on the equation (equations can have annotations; or one could even consider if for the entire model) stating that unit-checking is more forgiving here. Obviously there are some details to investigate: how forgiving, what annotation, etc? |
I agree that the language could be better at supporting this case. However, for now I suggest that we do it in the elaborate but clearly unit-correct way in the MSL, and initiate a language discussion about how to better support this case in the future. |
I would prefer to do it in the other way - since we haven't completed the unit-discussion for the language and it seems logical that specifying the unit-checking goes together with a mechanism for escaping it. In my view MSL is both a library that people can use, and a "template" for how to write models - and for correlations such as this I believe we need a readable good and consistent way to handle it. Additionally there are risks with rewriting equations - especially to make them "unit-free" as it may hide typos (new and old). |
Just a note regarding f_Theta - as far as I can tell the paper Crossley and Cook doesn't give it a unit. (The give unit for mass "g", and pressure "bar", etc.) Trying the proposed unit-rules indicates a problem in the model - even after removing the correlation (that we know is unit-sloppy - an alternative would be to implement support for disableUnitCheck-annotation, but simply removing the equation was faster). However, changing In IntakeManifold a similar removal of |
The following equation doesn't pass unit checking:
N
is expressed in"rad/s"
andP_m
in"bar"
so- 0.0337 * N * P_m ^ 2
and0.0001 * N ^ 2 * P_m
don't have the same unit, for instance, despite being added.The text was updated successfully, but these errors were encountered: