Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#3939 closed defect (fixed)

unitChecking does not always find mismatched units

Reported by: Christoph <buchner@…> Owned by: Lennart Ochel
Priority: high Milestone: 1.11.0
Component: Backend Version:
Keywords: Cc: Adrian Pop

Description

I just found out that unitChecking does not find some inconsistent units. I had inexplicable simulation problems, until I found out that I had a term in an equation that was accidentally copied in.

The thing is, the term made the equations dimensionally inconsistent, and I have switched on unitChecking to prevent exactly these mistakes. However, this was not picked up when simulating in OMEdit.

Minimal repro example: (equations were enabled one at a time to narrow down the issue)

model unitCheckingFailure
  parameter Modelica.SIunits.MolarEnthalpy deltaH = -200e3 "J/mol";
  parameter Modelica.SIunits.MolarEntropy deltaS = -100 "J/(mol.K)";
  parameter Modelica.SIunits.Temperature T0 = 1 "K";
  Modelica.SIunits.Temperature T "K";
  //Modelica.Constants.R: J/(mol.K)
equation

  //-1/T0 = deltaH - T; //this is wrong and gets picked up, so unit checking works

  //-1/T0 = deltaH/T - deltaS; //(correctly) does not get complaints
  //the following also (correctly) does not get complaints
  //-1/T0 = deltaH/(Modelica.Constants.R*T) - deltaS/(Modelica.Constants.R); 
  
  //the following, however, is dimensionally wrong, but not detected! 
  // compare last term to above equation, additional *T
  -1/T0 = deltaH/(Modelica.Constants.R*T) - deltaS/(Modelica.Constants.R*T); 

  //if we remove Modelica.Constants.R, it starts to detect the unit mismatch!
  //-1/T0 = deltaH/(T) - deltaS/(T); 
end unitCheckingFailure;

In OMEdit-Options-Simulation, I have +d=initialization --preOptModules+=unitChecking, and the above analysis confirms that it works in principle.
This is on OM nightly OMEdit v1.10.0-dev-134-gb85235d, OpenModelica v1.10.0-dev-572-gcde23af.

Change History (9)

comment:1 by Lennart Ochel, 9 years ago

Owner: changed from somebody to Lennart Ochel
Status: newaccepted

comment:2 by Lennart Ochel, 9 years ago

Cc: Adrian Pop added

That is exactly the reason why it is still „experimental“ and disabled by default. The issue is not, that the unit checking module does it wrong, but it gets previously simplified expressions. The solution to this is to move the unit check module into the front end. This is already scheduled, but unfortunately it will take some time, since the new front end needs to be integrated first.

This is the equation that is passed to the unit checking module:

1.0 / T0 = 0.1202723958085647 * (deltaH - deltaS) / T   [dynamic]

comment:3 by Christoph <buchner@…>, 9 years ago

Ah, I was not aware that unit checking is still experimental (the user guide does not mention that fact).

This is the equation that is passed to the unit checking module:
1.0 / T0 = 0.1202723958085647 * (deltaH - deltaS) / T [dynamic]

If unitcheck checks this simplified expression, it still should realize that you can't do (deltaH - deltaS), as the dimensions don't match (J/mol - J/(mol.K)), so maybe it does something wrong after all?

in reply to:  3 comment:4 by Lennart Ochel, 9 years ago

Replying to Christoph <buchner@…>:

Ah, I was not aware that unit checking is still experimental (the user guide does not mention that fact).

I will add a note to the users guide.

Replying to Christoph <buchner@…>:

If unitcheck checks this simplified expression, it still should realize that you can't do (deltaH - deltaS), as the dimensions don't match (J/mol - J/(mol.K)), so maybe it does something wrong after all?

You are right. I will have a look.

comment:5 by Christoph <buchner@…>, 8 years ago

@lochel: Did you manage to have a look at this?

comment:6 by Lennart Ochel, 8 years ago

@Christoph: Yes, it is a bug. I just pushed a fix for it: OpenModelica/OMCompiler#1087

in reply to:  6 comment:7 by Lennart Ochel, 8 years ago

Resolution: fixed
Status: acceptedclosed

comment:8 by Lennart Ochel, 8 years ago

Component: *unknown*Backend
Milestone: Future1.11.0

comment:9 by Christoph <buchner@…>, 8 years ago

Thank you, I can confirm this being fixed!

Note: See TracTickets for help on using tickets.