Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Create Rule22-42.md #1520
base: feature/ashrae9012022
Are you sure you want to change the base?
Create Rule22-42.md #1520
Changes from 1 commit
83afcdd
aee3878
1de24bf
c709250
20ddb2d
5dd22dc
4cfa6b9
6ecd056
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we include an applicability check for the baseline HVAC system type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. It overcomplicates things here, and if the user has issues with the HVAC system type, they'll be notified elsewhere. If we do a check here, it just means that if a user is using the RCT to validate a model then they have to do an extra iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weilixu I imagine that the actual implementation would use table lookups here. Is it clear enough from this RDS without using table lookups?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weilixu do we need to create these lookup tables or do they exist for OS Standards?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@supriyagoel We do not have plan to develop 2022 in OS Standards in FY25.
In the case that if we are developing this feature, it would be a look up table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This formula looks correct cut recommend upsating ect and cwt nomenclature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything has been updated to be ecwt and cwt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typical abbreviation for chilled water is CHW so I recommend that any variable that pertains to chilled water use CHW and any variable that represents condensing water be CW. Looks like they both use CW regardless of whether they are chilled or condenser water. This is a suggestion - feel free to ignore but I found it was a little confusing with CW for both. Maybe use the nomenclature in 90.1 with CHWT and ECT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on above on line 34 I think this is supposed to be for chwt in expected_chwt_temps"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
everything has been updated to be ecwt and cwt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typical abbreviation for chilled water is CHW so I recommend that any variable that pertains to chilled water use CHW and any variable that represents condensing water be CW. Looks like they both use CW regardless of whether they are chilled or condenser water. This is a suggestion - feel free to ignore but I found it was a little confusing with CW for both. Maybe use the nomenclature in 90.1 with CHWT and ECT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, I see what you're looking for now. Using CHWT and ECT now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may just be tired but I am not following how this load is getting retrieved from this. I don't think you are looping through power_validation_point objects at this point. Looks like you are looping through a list of power validation point results. I am thinking the loads also need to be included in the dictionary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load is a property of ChillerPowerValidationPoint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay but I do not see how you are getting to this with the current logic. How are you able to reference the power_validation_point, you are not looping through them anywehre and there can be multiple associated with the chiller. power_validation_point is an object so I am not following how you are able to reference it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right - I made changes also on line 50, so that power_validation_pts_dict[dict_key] references power_validation_points, not a list of power_validation_point results. Then we can get both the result and load on lines 75 and 76
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we allow some deviation? I can imagine the plr not being exactly 0.25 for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. I added a note to the dev team to accept a deviation of 0.01 - which is based on the understanding that if a number is given to two decimal places, there is a deviation of 0.01 allowed (PLRs are 0.25, etc)