Skip to content

Fix #7225 - Add Year field in SQL for DaylightMapHourlyReports#7235

Merged
Myoldmopar merged 6 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_7225_SQLDaylightingYearField
Jul 17, 2019
Merged

Fix #7225 - Add Year field in SQL for DaylightMapHourlyReports#7235
Myoldmopar merged 6 commits intoNatLabRockies:developfrom
jmarrec:PR_opened/Fix_7225_SQLDaylightingYearField

Conversation

@jmarrec
Copy link
Copy Markdown
Contributor

@jmarrec jmarrec commented Mar 20, 2019

Pull request overview

Fix #7225 - Add Year field in SQL for DaylightMapHourlyReports

I also have uploaded a supporting file to EnergyPlusDevSupport/DefectFiles/7000s/7225 if you want to look at a multiyear sql output.

Work Checklist

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

  • Title of PR should be user-synopsis style (clearly understandable in a standalone changelog context)
  • At least one of the following appropriate labels must be added to this PR to be consumed into the changelog:
    • Defect: This pull request repairs a github defect issue. The github issue should be referenced in the PR description
    • Refactoring: This pull request includes code changes that don't change the functionality of the program, just perform refactoring
    • NewFeature: This pull request includes code to add a new feature to EnergyPlus
    • Performance: This pull request includes code changes that are directed at improving the runtime performance of EnergyPlus
    • DoNoPublish: This pull request includes changes that shouldn't be included in the changelog

Review Checklist

This will not be exhaustively relevant to every PR.

  • Functional code review (it has to work!)
  • If defect, results of running current develop vs this branch should exhibit the fix
  • CI status: all green or justified
  • Performance: CI Linux results include performance check
  • Unit Test(s)
  • C++ checks:
    • Argument types
    • If any virtual classes, ensure virtual destructor included, other things
  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults
    • Open windows IDF Editor with modified IDD to check for errors
    • If transition, add to input rules file for interfaces
    • If transition, add transition source
    • If transition, update idfs
  • If new idf included, locally check the err file and other outputs
  • Required documentation updates?
  • ExpandObjects changes?
  • If output changes, including tabular output structure, add to output rules file for interfaces

@nrel-bot
Copy link
Copy Markdown

@jmarrec it has been 20 days since this pull request was last updated.

@nrel-bot
Copy link
Copy Markdown

@jmarrec it has been 7 days since this pull request was last updated.

1 similar comment
@nrel-bot
Copy link
Copy Markdown

@jmarrec it has been 7 days since this pull request was last updated.

@nrel-bot
Copy link
Copy Markdown

nrel-bot commented May 3, 2019

@jmarrec it has been 8 days since this pull request was last updated.

@nrel-bot-2
Copy link
Copy Markdown

@jmarrec it has been 13 days since this pull request was last updated.

@nrel-bot-2c
Copy link
Copy Markdown

@jmarrec it has been 7 days since this pull request was last updated.

@nrel-bot-2c
Copy link
Copy Markdown

@jmarrec it has been 8 days since this pull request was last updated.

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Jun 3, 2019
@jmarrec
Copy link
Copy Markdown
Contributor Author

jmarrec commented Jun 5, 2019

@Myoldmopar could you suggest a reviewer please?

@Myoldmopar
Copy link
Copy Markdown
Member

@macumber do you envision this causing any issues on OS side?

@macumber
Copy link
Copy Markdown

Don't think so, @jmarrec could check if there is :-)

@jmarrec
Copy link
Copy Markdown
Contributor Author

jmarrec commented Jul 17, 2019

@Myoldmopar This actually coming from OpenStudio's side, I noticed this daylightmap table was missing the year field while adding the year field to other tables now that it was available, cf NatLabRockies/OpenStudio#3453

More specifically, I should mostly have to revert that commit to get OS to make use of this PR: NatLabRockies/OpenStudio@79ecaa3


// We need DataGlobals::CalendarYear, and not DataEnvironment::Year because
// otherwise if you run a TMY file, you'll get for eg 1977, 1981, etc
SQYear = DataGlobals::CalendarYear;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


sqlite->createSQLiteDaylightMap(
MapNum, SQMonth, SQDayOfMonth, HourOfDay, IllumMap(MapNum).Xnum, XValue, IllumMap(MapNum).Ynum, YValue, IllumValue);
MapNum, SQYear, SQMonth, SQDayOfMonth, HourOfDay, IllumMap(MapNum).Xnum, XValue, IllumMap(MapNum).Ynum, YValue, IllumValue);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, adding a year so it can report.

const std::string daylightMapHourlyReportsTableSQL = "CREATE TABLE DaylightMapHourlyReports ( "
"HourlyReportIndex INTEGER PRIMARY KEY, "
"MapNumber INTEGER, Month INTEGER, DayOfMonth INTEGER, Hour INTEGER, "
"MapNumber INTEGER, Year INTEGER, Month INTEGER, DayOfMonth INTEGER, Hour INTEGER, "
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course the SQL will need a new argument

sqliteExecuteCommand(daylightMapHourlyReportsTableSQL);

const std::string daylightMapHourlyTitleInsertSQL = "INSERT INTO DaylightMapHourlyReports VALUES(?,?,?,?,?);";
const std::string daylightMapHourlyTitleInsertSQL = "INSERT INTO DaylightMapHourlyReports VALUES(?,?,?,?,?,?);";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And a placeholder

sqliteBindInteger(m_daylightMapHourlyTitleInsertStmt, 3, month);
sqliteBindInteger(m_daylightMapHourlyTitleInsertStmt, 4, dayOfMonth);
sqliteBindInteger(m_daylightMapHourlyTitleInsertStmt, 5, hourOfDay);
int b = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it's reasonable to use a counter here instead of manually persisting the order.


ASSERT_EQ(1ul, daylightMapHourlyReports.size());
std::vector<std::string> daylightMapHourlyReport0{"1", "1", "7", "21", "5"};
std::vector<std::string> daylightMapHourlyReport0{"1", "1", "2005", "7", "21", "5"};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me.

@Myoldmopar Myoldmopar merged commit 49bedc3 into NatLabRockies:develop Jul 17, 2019
@jmarrec jmarrec deleted the PR_opened/Fix_7225_SQLDaylightingYearField branch July 17, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL Table DaylightMapHourlyReports doesn't have a "Year" field

9 participants