Skip to content
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

Compiling boptest test case "multizone_office_simple_hydronic" #1343

Closed
dhblum opened this issue Dec 12, 2023 · 20 comments
Closed

Compiling boptest test case "multizone_office_simple_hydronic" #1343

dhblum opened this issue Dec 12, 2023 · 20 comments

Comments

@dhblum
Copy link
Contributor

dhblum commented Dec 12, 2023

This issue is to address problems with compiling the boptest emulator multizone_office_simple_hydronic.

@jelgerjansen This is as discussed in the IBPSA Project 2 meeting.


  1. Compiling with OCT results in these errors:
Error at line 187, column 3, in file '/home/dhbubu20/git/ideas/IDEAS/IDEAS/Buildings/Components/Interfaces/RectangularZoneTemplateInterface.mo':
  The component mSenFac is declared multiple times and differs from other declaration(s) with the same name:
    they have min attribute expressions with different values.
  This difference was found when comparing the components declared:
    in  IDEAS.Buildings.Components.Interfaces.PartialZone, inherited into IDEAS.Buildings.Components.Interfaces.RectangularZoneTemplateInterface, then IDEAS.Buildings.Components.RectangularZoneTemplate
    and IDEAS.Buildings.Components.Interfaces.RectangularZoneTemplateInterface, inherited into IDEAS.Buildings.Components.RectangularZoneTemplate

Error at line 42, column 5, in file '/home/dhbubu20/git/ideas/IDEAS/IDEAS/Fluid/Actuators/Valves/Simplified/ThreeWayValveMotor.mo':
  The component m_flow_small is declared multiple times and differs from other declaration(s) with the same name:
    they have quantity attribute expressions with different values.
  This difference was found when comparing the components declared:
    in  IDEAS.Fluid.Interfaces.PartialTwoPortTransport, inherited into IDEAS.Fluid.Movers.BaseClasses.IdealSource, then IDEAS.Fluid.Actuators.Valves.Simplified.ThreeWayValveMotor.IdealSource
    and IDEAS.Fluid.Actuators.Valves.Simplified.ThreeWayValveMotor.IdealSource

Error at line 45, column 5, in file '/home/dhbubu20/git/ideas/IDEAS/IDEAS/Fluid/Actuators/Valves/Simplified/ThreeWayValveMotor.mo':
  The component show_T is declared multiple times and differs from other declaration(s) with the same name:
    they have different values for the HideResult annotation.
  This difference was found when comparing the components declared:
    in  IDEAS.Fluid.Interfaces.PartialTwoPortTransport, inherited into IDEAS.Fluid.Movers.BaseClasses.IdealSource, then IDEAS.Fluid.Actuators.Valves.Simplified.ThreeWayValveMotor.IdealSource
    and IDEAS.Fluid.Actuators.Valves.Simplified.ThreeWayValveMotor.IdealSource

  1. Compiling with Dymola results in these errors (attached the full error log which is many repeats of the same):
Screen Shot 2023-12-12 at 4 03 37 PM

dymola_error_log.txt

@jelgerjansen
Copy link
Contributor

jelgerjansen commented Dec 13, 2023

Hi @dhblum @icupeiro

Let me comment on the Dymola errors first. When I compile the model using Dymola 2023x, I don't get the errors you encounter. I am however able to reproduce your errors when I perform a pedantic check in Dymola 2023x. I presume you have some flag enabled in Dymola that performs a more strict compilation (maybe Advanced.PedanticModelica?).
A small comment regarding these errors:

  1. Component type specifier extends OldGlazing specified an obsolete type
    The glazing model that is used, was marked as obsolete by the previous developers because the product data is outdated. You could consider opting for another glazing type like IDEAS.Buildings.Data.Glazing.Ins2Ar2020.
  2. Connecting different kinds of variables
    This is a Modelica inconsistency that we solved by making the connectors expandable (see Invalid connection between connectors #1317). This is the fix I needed to revert in the BOPTEST branch to avoid compilation errors using OM and OCT. If you would use the IDEAS master branch, these errors vanish.
  3. You likely forget a semi-colon before the annotation. Therefore in class BuildingEmulators.BuildingSystem annotation Diagram, the annotation 'experiment' is not recognized at this place, but the annotation 'experiment' can be used in the top-annotation of a class.
    This seems like an inconsistency in the emulator model.
  4. Connecting two outside connectors(=connector in submodel) containing input components heating_cooling.heapro.TSet and heating_cooling.heapro.TSetIn.
    This seems like another inconsistency in the emulator model. @icupeiro I see that you created a TSetin input on top of the TSet input that is inherited from IDEAS.Fluid.Interfaces.PrescribedOutlet. Why do you need both?

When I compile the model using Dymola 2023x, I only get the latter error (see screenshot below), so that one needs to be solved for sure.
image

@jelgerjansen
Copy link
Contributor

Regarding the OCT errors, I think these are also warnings that are flagged by OCT due to some flag that is activated (similar to the pedantic check in Dymola). Nevertheless, it makes sense to fix these, because they indicate inconsistencies in our models.

  1. RectangularZoneInterface
    I tried to track down the obsolete/contradicting declarations of mSenFac, but strangely enough my first fix didn't not work: in IDEAS.Buildings.Components.Interfaces.RectangularZoneTemplateInterface I added final mSenFac=mSenFac in the extension of IDEAS.Buildings.Components.Interfaces.PartialZone, thinking this would overwrite all lower level mSenFac declarations.
    What does work is removing the declaration of mSenFac in RectangularZoneTemplateInterface, but then the default information for (mSenFac(min=0.1)=5) gets lost.
  2. ThreeWayValveMotor
    I think removing the declaration of m_flow_small and show_T in the IdealSource component declared within the ThreeWayValveMotor component will fix the problem. Because this tailor-made component extends from IDEAS.Fluid.Movers.BaseClasses.IdealSource, which also has a declaration for m_flow_small and show_T, OCT is throwing a warning/error.

@Mathadon do you have any idea why my initial 'fix' for problem 1 does not remove the 'multiple declaration' warning?
I already pushed a fix for the ThreeWayValveMotor in #1344)

@dhblum
Copy link
Contributor Author

dhblum commented Dec 13, 2023

Thanks a lot @jelgerjansen for your quick reply and help on this.

Regarding Dymola, indeed it was a pedantic check that caused those warnings to be errors. Without the pedantic check, I get what you show in your screen shot.

Regarding OCT, sounds like you're on a track to fixing in IDEAS. I need to do some searching to see what/if there's a pedantic flag.

@icupeiro
Copy link
Member

@dhblum @jelgerjansen I acknowledge the message but due to some EOY priorities that I need to meet I may take a bit of time to look at the issues

@icupeiro
Copy link
Member

icupeiro commented Jan 2, 2024

@jelgerjansen

  1. The model is already using Ins2Ar2020 glazing; I think the issue may have to do with the extension
  2. N/A
  3. I am not able to reproduce/see this one
  4. The model extends from IDEAS.Fluid.Interfaces.PrescribedOutlet, which has a conditional input given by 'use_TSet' parameter. Thus, to have a 'persistent' input I had to do the 'trick':
    if use_TSet then
        connect(TSet,TSetIn);
    else 
        TSetIn = 0;
    end if;

@jelgerjansen
Copy link
Contributor

jelgerjansen commented Jan 2, 2024

@icupeiro

  1. You're right, this is due to the default glazing type in RectangularZoneTemplate. I'll consider updating that one.
  2. NA
  3. I don't immediately see why the pedantic check is throwing an error here, but does not seem to be problematic.
  4. You might want to consider another formulation then if you want this to compile in Dymola (error also occurs in non-pedantic mode).

@icupeiro
Copy link
Member

icupeiro commented Jan 3, 2024

@jelgerjansen

I refactored the conditional input based on how is done with the boundary_pT model (lines 66-68). The code now looks like this:

    connect(TSet,TSetIn);
    if not use_TSet then       
        TSet = 0;
    end if;

Let me know if this solve the problem, but based on your error message I have a suspicion that the problem underlies in connecting two RealInput signals together. No worries at all, if it is not solved I can try to redefine TSetIn as a Real parameter instead of a RealInput connector (but unfortunately I do not have a Dymola license to test it out). I will push this change into my project1-boptest branch fork (see pull request related to this emulator)

@jelgerjansen @dhblum

I am able to compile the model using OpenModelica version 1.23.0~dev-206-g00d3636 and the current Buildings master and IDEAS BOPTEST branches as for today. A yearly simulation with Euler integrator (15s step) takes about 15-20 minutes, a bit longer with CVodes

@jelgerjansen
Copy link
Contributor

@icupeiro this fix indeed doesn't solve the problem.
The implementation however seems similar to the use of shaCoe (use_shaCoe, shaCoe_in, ...) in the solar collector model (Buildings.Fluid.SolarCollectors). That model might provide an alternative implementation for your model.

Good to know that the model now compiles in OM.
Apart from some warnings, the problems with the use of IDEAS in the emulator seem to be solved right now.

@icupeiro
Copy link
Member

icupeiro commented Jan 4, 2024

@jelgerjansen

pushed a new formulation based on those models (still not sure it will work).

Since it only affects the compilation in Dymola (commercial software) I do not know how much should we worry about this. @dhblum let me know if this is a hard constraint

@jelgerjansen
Copy link
Contributor

@icupeiro the errors remain:
image

@icupeiro
Copy link
Member

icupeiro commented Jan 4, 2024

@dhblum

Had a chat with @jelgerjansen and solved the Dymola compilation issues. The model now compiles with Dymola

@dhblum
Copy link
Contributor Author

dhblum commented Jan 4, 2024

This is all great to see, thank you both for continued efforts. I confirm the model compiles and simulates ok (at least for 10 days) with OM (v1.22.1 and v1.23-dev). I was able to also compile a CS FMU with OM v1.23-dev, but the FMU fails to simulate when I tried with pyfmi (via OCT).

I couldn't find any sort of pendantic flag in OCT compiler, so fixing the errors with Dymola might help fix the errors with OCT as well. I will test with these updates and report back.

@jelgerjansen
Copy link
Contributor

jelgerjansen commented Jan 5, 2024

@dhblum are you getting the same errors with OCT as reported initially (multiple declarations)?

This branch should already solve the problem with the ThreeWayValveMotor. For the multiple declaration error coming from RectangularZoneInterface: I'll try and have another look to fix it (my first attempts were not successful).

@dhblum
Copy link
Contributor Author

dhblum commented Jan 5, 2024

Here we go. I pulled the latest for the emulator model and used the latest IDEAS branch issue1343_BoptestMultizoneOffice.

Compiling and simulating in Dymola without pedantic mode works (but see below). Compiling and simulating in OM works (but see below).

Compiling in OCT fails with these errors (@jelgerjansen the problem with ThreeWayValveMotor is indeed fixed, but still issues with RectangularZoneInterface and also now with the conditional TSetIn and TSet):

Error at line 25, column 36, in file '/home/dhbubu20/git/ibpsa/project1-boptest-iago/project1-boptest/testcases/multizone_office_simple_hydronic/models/BuildingEmulators/Components/IdealProduction.mo':
  The component TSetIn is conditional: Access of conditional components is only valid in connect statements

Error at line 27, column 62, in file '/home/dhbubu20/git/ibpsa/project1-boptest-iago/project1-boptest/testcases/multizone_office_simple_hydronic/models/BuildingEmulators/Components/IdealProduction.mo':
  The component TSetIn is conditional: Access of conditional components is only valid in connect statements

Error at line 36, column 5, in file '/home/dhbubu20/git/ibpsa/project1-boptest-iago/project1-boptest/testcases/multizone_office_simple_hydronic/models/BuildingEmulators/Components/IdealProduction.mo':
  The component TSet is conditional: Access of conditional components is only valid in connect statements

Error at line 36, column 10, in file '/home/dhbubu20/git/ibpsa/project1-boptest-iago/project1-boptest/testcases/multizone_office_simple_hydronic/models/BuildingEmulators/Components/IdealProduction.mo':
  The component TSetIn is conditional: Access of conditional components is only valid in connect statements

Error at line 38, column 9, in file '/home/dhbubu20/git/ibpsa/project1-boptest-iago/project1-boptest/testcases/multizone_office_simple_hydronic/models/BuildingEmulators/Components/IdealProduction.mo':
  The component TSet is conditional: Access of conditional components is only valid in connect statements

Error at line 187, column 3, in file '/home/dhbubu20/git/ideas/IDEAS/IDEAS/Buildings/Components/Interfaces/RectangularZoneTemplateInterface.mo':
  The component mSenFac is declared multiple times and differs from other declaration(s) with the same name:
    they have min attribute expressions with different values.
  This difference was found when comparing the components declared:
    in  IDEAS.Buildings.Components.Interfaces.PartialZone, inherited into IDEAS.Buildings.Components.Interfaces.RectangularZoneTemplateInterface, then IDEAS.Buildings.Components.RectangularZoneTemplate
    and IDEAS.Buildings.Components.Interfaces.RectangularZoneTemplateInterface, inherited into IDEAS.Buildings.Components.RectangularZoneTemplate

I see a problem where the simulation results for Dymola and OM do not agree. Here are results for the first 10 days of simulation (I get the same results respectively in each tool whether I use cvode or euler with a timestep of 15s). In OM, the temperature for zone 2 is not meeting setpoint. I haven't done further diagnostics at this point.

Dymola
Screen Shot 2024-01-05 at 9 24 56 AM

OM
Screen Shot 2024-01-05 at 9 25 34 AM

@jelgerjansen
Copy link
Contributor

jelgerjansen commented Jan 5, 2024

@icupeiro @dhblum it seems that the Dymola fix causes errors with OCT, so we'll need another solution that works for both...
Is it okay that we move this discussion, together with differences between Dymola and OM simulation results, to another issue (probably somewhere in the boptest repo)?

I'll keep this issue open to fix the last IDEAS error in the model (RectangularZoneInterface) and close it once the fix is added to PR #1344 and merged in the IDEAS boptest and master branch.

@dhblum
Copy link
Contributor Author

dhblum commented Jan 5, 2024

Sounds good @jelgerjansen to compartmentalize, your suggestion is reasonable. I'll report the issues related to Dymola/OCT compilation and Dymola/OM results in ibpsa/project1-boptest#465. Tag me when you resolve #1344 or if you need additional testing there.

@jelgerjansen
Copy link
Contributor

I tried to solve the 'multiple declaration' error by adding mSenFac=mSenFac in the extension of PartialZone in the model RectangularZoneTemplateInterface, but this gives an error in Dymola:
image

I therefore removed the declaration of mSenFac in RectangularZoneTemplateInterface and added mSenFac(min=0.1) in the extension of PartialZone (since the minimum is the only difference between both declarations).

@dhblum can you check if this fixes the OCT error? (see this branch)
@Mathadon do you agree with this approach?

@dhblum
Copy link
Contributor Author

dhblum commented Jan 5, 2024

@jelgerjansen Yes your update removes the error related to RectangularZoneTemplateInterface from OCT!

@Mathadon
Copy link
Member

@jelgerjansen yup sounds good!

@jelgerjansen
Copy link
Contributor

Closed by #1344 and #1349

jelgerjansen added a commit that referenced this issue Jan 25, 2024
…s of PR 1343 and 1349 (issue #1343) in the master branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants