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

Site:Location should not be overridden by weather file #10579

Closed
3 tasks
shorowit opened this issue Jun 20, 2024 · 2 comments · Fixed by #10586
Closed
3 tasks

Site:Location should not be overridden by weather file #10579

shorowit opened this issue Jun 20, 2024 · 2 comments · Fixed by #10586
Assignees
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) LowComplexityApproved Used for subcontractor defect complexity requests NewFeatureRequest This "issue" is a new feature request, not a defect report

Comments

@shorowit
Copy link
Contributor

Issue overview

As described in the Site:Location documentation:

The location class describes the parameters for the building’s location. Only one location is allowed. Weather data file location, if it exists, will override any location data in the IDF. Thus, for an annual simulation, a Location does not need to be entered."

This doesn't make any sense to me. There are legitimate situations where a user has entered a Site:Location that significantly deviates from the weather station. An obvious example is that the building's site is at a high elevation in the mountains, but the nearest weather station is at lower elevation. Or the building is in one time zone but the nearest weather station is just over the state's border in another time zone.

I agree that Site:Location should be allowed to be omitted, such that the weather file data is used, but if the Site:Location is provided, it should take precedence over the weather file.

Details

Some additional details for this issue (if relevant):

  • Platform (Operating system, version)
  • Version of EnergyPlus (if using an intermediate build, include SHA)
  • Unmethours link or helpdesk ticket number

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Defect file added (list location of defect file here)
  • Ticket added to EnergyPlus Defect Complexity (Github Project)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@mjwitte
Copy link
Contributor

mjwitte commented Jun 20, 2024

Given the long history of the current behavior, I would suggest adding a field to Site:Location to optionally override the epw location.

@mjwitte mjwitte added the NewFeatureRequest This "issue" is a new feature request, not a defect report label Jun 20, 2024
@RKStrand
Copy link
Contributor

@mjwitte I was thinking the same thing as I read this. A new field in the Site:Location syntax that allows one to override the epw file seems like a good solution.

@RKStrand RKStrand self-assigned this Jun 25, 2024
@RKStrand RKStrand added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Jun 25, 2024
@RKStrand RKStrand added this to the EnergyPlus 24.2 IOFreeze milestone Jun 25, 2024
RKStrand added a commit that referenced this issue Jun 27, 2024
Implementation of a new feature to allow the user to allow the user to still use the Site:Location designation even when a weather file exists.  Currently, if a weather file exists, it always overwrite the Site:Location information.  This allows the user to choose.  Commit includes modifications to the IDD, code to read in the new input and handle it properly within the code, and a new input file that shows that the Site:Location information is kept when requested by the user.  Expecting no changes in existing IDFs since the default is to maintain the status quo from previous editions.
RKStrand added a commit that referenced this issue Jun 27, 2024
Forgot to do the clang format check before making a commit. Oops...
RKStrand added a commit that referenced this issue Jun 28, 2024
Addition of a unit test that exercises two different subroutines impacted by the work for this new feature.  Also, documentation additions/revisions were made.  This commit is a candidate PR.
RKStrand added a commit that referenced this issue Jun 28, 2024
Forgot to update CMakeLists.txt file for the new input file
RKStrand added a commit that referenced this issue Jun 28, 2024
The last commit introduced a logic error that caused several diffs in various files.  This is corrected here.
RKStrand added a commit that referenced this issue Jun 28, 2024
Got rid of an unnecessary error message (also required modifications to unit test) and corrected file name shown in the comments of the new file.  Also added some notes to the new IDF to explain what it's purpose is.
@Myoldmopar Myoldmopar added the LowComplexityApproved Used for subcontractor defect complexity requests label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) LowComplexityApproved Used for subcontractor defect complexity requests NewFeatureRequest This "issue" is a new feature request, not a defect report
Projects
None yet
4 participants