Issue3438 d xdehumidifier #3475
Conversation
Add the unit test results
Addressed inline comments
Issue3438 d xdehumidifier
@terrancelu92: I had a quick look and everything seems to be in order. Can you please review the changes and let @JayHuLBL know if all the changes are expected? Thanks! |
|
@karthikeyad-pnnl @terrancelu92 Is it possible to downgrade the IDF file, to 9.6? |
…er_mergeFix Issue3438 d xdehumidifier merge fix
| "Temperature sensor" | ||
| annotation (Placement(transformation(extent={{60,50},{80,70}}))); | ||
|
|
||
| Modelica.Units.SI.MassFraction XOut |
There was a problem hiding this comment.
Do we have to calcuate the XOut here? I also see that we have the sensor senMasFra and heaFloSen. Are they required by the model itself? I see that the values are not used anywhere else.
There was a problem hiding this comment.
@JayHuLBL Thanks for catching this! I did another review of the code, and realized both XOut and senMasFra are not required. However, we are using heaFloSen to validate the heat transfer of the component within the validation model. It will be useful to identify any errors introduced in the calculation, via CI testing. Please refer to #3788. Thanks!
… issue3438_DXdehumidifier_mergeFix
…er_mergeFix Issue3438 d xdehumidifier merge fix
| @@ -0,0 +1,38 @@ | |||
| within Buildings.Fluid.Humidifiers.Data; | |||
There was a problem hiding this comment.
It would be better if we could have a set of example data in the package. For example, see Buildings.Fluid.HeatPumps.Data.EquationFitReversible
There was a problem hiding this comment.
@JayHuLBL We actually have an example in the Validation package (Humidifiers.Validation.Data). Should we move it to this package?
There was a problem hiding this comment.
@JayHuLBL Is the data from the EnergyPlus reference?
There was a problem hiding this comment.
Yes. The parameter values came from the EPlus model that was used for the validation process
There was a problem hiding this comment.
Please add it to the Data package
…er_mergeFix Issue3438 d xdehumidifier merge fix
…/modelica-buildings into issue3438_DXdehumidifier
This closes #3438.
@karthikeyad-pnnl We are trying to merge the change to master. Would you please take another look to check if the changes are expected?