-
Notifications
You must be signed in to change notification settings - Fork 58
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
Issue996 hydronictest #997
Conversation
- added occupancy schedule - assigned units to inputs and outputs - finite heater capacity - added pressure drop for radiator
I emailed to you Review 2 using BOPTEST review guide. Discussion of any additional remaining features forthcoming soon. |
So, shall I still wait before looking at it then? |
@damienpicard indeed no need to merge/review this yet |
I think PI reference should equal setback temperature, and system sized appropriately to meet setpoints. Maybe not immediately, but within a reasonable time. |
Actually, let me rephrase a bit. I agree with you for the most part. I think I'm just making a stronger preference for increasing the system size to meet setpoints, rather than assume that the system is undersized for the baseline controller that would likely come with such a system. To me, this assumes some design flaw and arbitrarily puts the baseline controller at a disadvantage. |
So define comfort at 16 C, PI reference at 16 C (+ whatever deadband), increase system size by ~30%? |
… baseline controller
…oller to be after the overwrite block for reference governor controllers to have effect.
Increasing nominal heating capacity of boiler and radiators from 3 kW to 5 kW seems to work just fine. I've also decreased proportional constant and deadband from 0.2 degC to 0.1 degC as seems to improve the baseline. I've updated #1146 and the testcase documentation with these changes. These are the simulation results: If you agree with the changes and have no further comments I'll update ibpsa/project1-boptest#201 by re-compiling the model and updating references. After that the testcase should be ready to merge. |
The new results look good to me! I don't have further comments on them. Thanks @JavierArroyoBastida! |
Looks good indeed :) Is this ready for a review before merging? |
Great! I'd say the model is ready for review but the BOPTEST case still needs an update. I'll let you know once that's done for a complete review. |
Issue996 hydronictest add PI controller
This is ready to be merged once the unit tests pass. |
issue996 hydronictest
this is for #996
further revisions are still required
@dhblum can you comment further on the remaining features that still have to be added?