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

Addressed Documentation Issues - April-August 2016 #6463

Merged
merged 7 commits into from
Feb 14, 2018

Conversation

Nigusse
Copy link
Contributor

@Nigusse Nigusse commented Feb 5, 2018

Issue #5603

Pull request overview

Documentation update and Idd changes. Below are the list of changes:

  • Equivalent Layer Optical Model, equation symbols update
  • corrected typo in the ASHRAE_2005_HOF_Materials.idf data set
  • added a statement in I/O and Eng. Ref that WindowProperty:ShadingControl is not supported
  • corrected typo in five example files
  • Added People object Fraction Radiant default of 0.3 in the I/O ref document
  • added Ground exterior boundary condition saturated air assumption in HMAT model Eng Ref docs
  • Coil:Cooling:Water default heat exchanger configuration in I/O Ref changed to match the idd
  • corrected equations for Curve:ExponentialDecay and Curve:ExponentialSkewNormal in I/O Ref
  • corrected equation for Sky emissivity in the Auxiliary Programs doc
  • verified the equation for EIRfplr for Chiller:Electric:EIR in Eng. Ref
  • corrected the AirTerminal:SingleDuct:Mixer image275.png and image276.png in I/O Ref
  • corrected the GroundHeatExchanger:Surface image205.png and image206.png in IORef
  • corrected the default "Control Algorithm" for AvailabilityManager:OptimumStart in IORef
  • corrected reference for "Glass Optical Properties Conversion" in WindowMaterial:Glazing in I/O Ref
  • added a missing note for Number of Stepped Control Steps field in Daylighting:Controls in I/O ref
  • corrected VRF Heat Pump Heating Electric Power (and Consumption) output variable description

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

Review Checklist

  • IDD changes:
    • Verify naming conventions and styles, memos and notes and defaults
    • Open windows IDF Editor with modified IDD to check for errors
  • Documentation changes in place
  • Changed docs build successfully

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 6, 2018

@Myoldmopar I am not able to trace the reason for Ubuntu build failure. Can you please check and let me know? Thanks.

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 6, 2018

@Myoldmopar I cannot see the build failure problem locally. Can you please try to kick off Linux build and let me know which one is the offending equation?

@Myoldmopar
Copy link
Member

@Nigusse I found the build problem. When running LaTeX locally, I found the LaTeX Error: string in the output.

! LaTeX Error: Bad math environment delimiter.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...                                              
                                                  
l.2404 ...ange from $\theta = 0$ to $\theta = 90\(
                                                  ^{o}\) (ASHRAE 1311-RP). T...

[420] [421] [422]
! Extra }, or forgotten $.
l.2515 ...at normal incidence, A\(_{o}\) = {$\tau}
                                                  \(_{bb}\) ($\theta = 0$), ...

Look at l.2404 in that snippet. In the theta = 90 part, you open up math mode using $, but then you try to open up math mode again using (. Fail. 👍

I pushed up a fix for one of them, but I actually just noticed the second one there. Looks to be the same problem. I'll push that one up also.

@Myoldmopar
Copy link
Member

Alright, if those fix the Ubuntu error then this can merge in.

@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 13, 2018

@Myoldmopar Thank you for fixing the building problem.

@Myoldmopar
Copy link
Member

Oh come on it's still there. Ugh...rebuilding locally.

@Myoldmopar
Copy link
Member

@Nigusse OK, now they should all be cleaned. A full build of all doc files touched on this branch reveals no more instances of "LaTeX Error" in the compiler output.

One thing that I think may have caused this is some confusion about braces. In LaTeX, escaping parentheses is actually invoking math mode, so you can easily write inline math:

something like this \(a_1\) so that will render like nice math

But if you are already inside math mode, it will cause a failure, so:

\begin{equation}
ThisWont \(a_1\) work
\end{equation}

To be clear, I like to use single dollar signs for entering inline math mode:

like $a_1$

Now, all that said, please take a look at my last commit for two reasons.

  1. Can you verify the lines I added are still showing the right form of those equations?
  2. When you are making changes to the docs to fix errors, please take a little time to clean up the LaTeX code. As you can see from the lines I changed, the original equations were awful, filled with nonsense that dates back to the Word->Markdown transition. In almost all our documentation, the math can be cleaned up in a major way. Just keep that in mind as you make more changes.

Thanks though, these look good now and this will make another good set of cleanup on the docs and input files. If CI is happy on Linux, I'll merge it.

@Myoldmopar Myoldmopar added this to the EnergyPlus 8.9.0 milestone Feb 14, 2018
@Myoldmopar Myoldmopar added the IDDChange Code changes impact the IDD file (cannot be merged after IO freeze) label Feb 14, 2018
@Myoldmopar Myoldmopar merged commit dc4f43d into develop Feb 14, 2018
@Myoldmopar Myoldmopar deleted the 154819972_issue#5603 branch February 14, 2018 10:59
@Nigusse
Copy link
Contributor Author

Nigusse commented Feb 14, 2018

@Myoldmopar Just check documentation in a local build. The equation for the curve Curve:ExponentialSkewNormal, in output reference on page 2270 is incorrect. It should have been Z2 = [(exp(C3x) * C4x) - C1] / C2, instead of C3, C4 was entered is used.

@Myoldmopar
Copy link
Member

Gah, OK. I will make an issue to resolve after IO freeze so it doesn't add to CI burden now. Thanks for finding it.

@Myoldmopar Myoldmopar mentioned this pull request Feb 14, 2018
@Myoldmopar
Copy link
Member

OK, new issue made #6476; carry on.

@mjwitte mjwitte added the Defect Includes code to repair a defect in EnergyPlus label Feb 22, 2018
@Myoldmopar
Copy link
Member

Closed via #6494

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 IDDChange Code changes impact the IDD file (cannot be merged after IO freeze)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants