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

Performance decrease from 8.5 to 8.6 #5984

Closed
1 of 3 tasks
EnergyArchmage opened this issue Jan 18, 2017 · 6 comments
Closed
1 of 3 tasks

Performance decrease from 8.5 to 8.6 #5984

EnergyArchmage opened this issue Jan 18, 2017 · 6 comments
Labels
PriorityLow This defect or feature has been declared low priority SeverityMedium This defect is medium severity, indicating moderate impact and generally no workaround available

Comments

@EnergyArchmage
Copy link
Contributor

EnergyArchmage commented Jan 18, 2017

Issue overview

User reports that execution time increased by factor of 9 between version 8.5 and version 8.6. Preliminary testing confirmed performance decrease. From monitoring stdout, appears to be in zone heat balance model because slow execution happens during zone sizing calculations as well as full HVAC simulation.

One thing to investigate is use ofDetailedSkyDiffuseModeling algorithm in ShadowCalculation, file has overlapping shading surfaces and maybe varying transmittance of shading surfaces.

Details

Some additional details for this issue (if relevant):

Checklist

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

defect files include both the original user files and a second pair that are fixed up. user had made small EMS changes between and apparently was using a modified IDD that changed a minimum. defect files that start out with 5984* are the fixed ones

  • Ticket added to Pivotal for defect (development team task)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@mbadams5
Copy link
Contributor

@EnergyArchmage I profiled the code and this is the call stack down the functions that are causing the performance regression.

Weight	Self Weight		Symbol Name
1.02 min  100.0%	0 s	 	energyplus-8.6.0 (28518)
1.02 min  100.0%	0 s	 	 Main Thread  0x77b6f
1.02 min   99.9%	0 s	 	  start
1.02 min   99.9%	0 s	 	   main
1.02 min   99.9%	0 s	 	    EnergyPlusPgm(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
1.01 min   99.4%	0 s	 	     EnergyPlus::SimulationManager::ManageSimulation()
1.01 min   99.3%	0 s	 	      EnergyPlus::SizingManager::ManageSizing()
59.23 s    97.0%	0 s	 	       EnergyPlus::HeatBalanceManager::ManageHeatBalance()
57.16 s    93.6%	0 s	 	        EnergyPlus::HeatBalanceManager::InitHeatBalance()
57.16 s    93.6%	0 s	 	         EnergyPlus::SolarShading::PerformSolarCalculations()
57.16 s    93.6%	0 s	 	          EnergyPlus::SolarShading::CalcPerSolarBeam(double, double, double)
57.15 s    93.6%	238.00 ms	 	   EnergyPlus::SolarShading::FigureSolarBeamAtTimestep(int, int)
56.91 s    93.2%	373.00 ms	 	    EnergyPlus::SolarShading::SHADOW(int, int)

Now, what I haven't figured out yet is why the shadow calculations are running many more iterations in 8.6 vs 8.5. I tried to look back through old PRs for 8.6 but nothing jumped out at me.

@EnergyArchmage
Copy link
Contributor Author

Thanks https://github.com/mbadams5 ! I think it likely to be this one #5741

@mbadams5
Copy link
Contributor

Considering the call stack and glancing at the code, I agree.

@EnergyArchmage
Copy link
Contributor Author

Note that user had ShadowCalculation's Calculation Method key set to AverageOverDaysInFrequency , but it may need key TimestepFrequency instead, when used with DetailedSkyDiffuseModeling.

@JasonGlazer
Copy link
Contributor

There is no doubt that fixing #5741 has some performance impacts on specific files.

@mjwitte mjwitte added PriorityLow This defect or feature has been declared low priority SeverityMedium This defect is medium severity, indicating moderate impact and generally no workaround available labels Feb 24, 2017
@mjwitte mjwitte added this to the EnergyPlus 8.7.0 milestone Feb 24, 2017
@mitchute
Copy link
Collaborator

mitchute commented Feb 4, 2020

Closing in lieu of the ongoing 10X work. We should open a new issue if there's a specific issue to address.

@mitchute mitchute closed this as completed Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PriorityLow This defect or feature has been declared low priority SeverityMedium This defect is medium severity, indicating moderate impact and generally no workaround available
Projects
None yet
Development

No branches or pull requests

6 participants