Issue680 update dependency bestest hydronic

[EttoreZ]

This pull request is for #680. Modelica code of BETEST_hydronic and BESTEST_hydronic_heat_pump was updated to NOT extend from model of IDEAS library, but instead duplicate it, to make test case Modelica code maintenance easier.

[EttoreZ]

One comment. I did not add the new wrapped.fmu since the code change does not affect results. This is to avoid further increasing the size repository.

[dhblum]

@jelgerjansen This PR proposes to move the code for the bestest_hydronic and bestest_hydronic_heat_pump test cases from IDEAS (and included in BOPTEST via Modelica extends), to having the models in the IBPSA repo and have IDEAS as a versioned dependency. This is the approach taken with other test cases in BOPTEST that depend on IDEAS and Buildings. We feel this would help decouple maintenance and future development of the models from IDEAS development and maintenance, especially for things that are not IDEAS-specific. A downside is that the test cases are no longer unit tested with your IDEAS development.

Let me know if this seems ok to you or if you see reasons we should keep the models within IDEAS, as was set up originally.

Note also that since the model code would be duplicated from an original distribution in IDEAS, I've included reference to the IDEAS license in the BOPTEST license as part of this PR.

[jelgerjansen]

Hi @dhblum, I think it is OK to move these models from IDEAS to BOPTEST. According to my knowledge, the main functionalities of both the component models in both models are also tested in other unit tests. Or do I miss something important here @Mathadon?

I think it would also be possible to just keep the models in IDEAS, right? They will just be out-of-sync with those of BOPTEST. Although I don't see a reason to keep them in IDEAS.

I noticed that the IDEAS v2.2.1 is referred to, but this was before the update of MSL to v4.0.0. Are you sure about the IDEAS version in the documentation?

[EttoreZ]

hey @jelgerjansen answering your second question. This version of the test case still uses Modelica 3.2.3 due to the legacy support of JModelica for compilation, hence IDEAS 2.2.1. We are moving away from JModelica and working to update all BOPTEST test cases to Modelica 4.0 and related version of the libraries (like IDEAS 3.0.0).

[dhblum]

Thanks @jelgerjansen. To your first question about unit tests: BOPTEST unit tests test the compilation and simulation of the full test case models with reference results produced and compared for BOPTEST KPI values and a selection of variable timeseries that are generated during the tests. I imagine the components of the models are unit tested in the IDEAS library, but you or @Mathadon may be able to confirm better.

[jelgerjansen]

@EttoreZ @dhblum thanks for the clarification :)
Please proceed with this PR, and we'll check internally if we keep the models in IDEAS, move them to an Obsolete package, or remove them.

[Mathadon]

I see no reason to object against the proposed route if the BOPTEST maintainers think this is the best way overall :)

[dhblum]

Thank you both @jelgerjansen and @Mathadon. We will proceed with this PR.

[dhblum]

@EttoreZ Please address my two comments.

[dhblum]

@EttoreZ Can you also address #625 in this PR?

[EttoreZ]

@EttoreZ Can you also address #625 in this PR?

Hey @dhblum I gave it a bit more thought. I think it makes more sense to integrate #625 with the min max PR for #658, since I am already updating the inputs for BETEST_hydronic and BESTEST_hydronic_heat_pump there, and it would be easier to merge them without conflicts. Is that ok?

[dhblum]

@EttoreZ Yes, that sounds fine. Thanks for thinking strategically about it. Then just let me know if/when this PR is ready.

[EttoreZ]

Ok perfect, @dhblum PR should be ready now and I will work on the min max PR.