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

revamp EpwDataPoint to use double attributes where appropriate #3701

Open
jmarrec opened this issue Oct 1, 2019 · 1 comment
Open

revamp EpwDataPoint to use double attributes where appropriate #3701

jmarrec opened this issue Oct 1, 2019 · 1 comment

Comments

@jmarrec
Copy link
Collaborator

jmarrec commented Oct 1, 2019

There's a lot of back and forth conversions unnecessarily happening because the attributes are std::string but it should be double (or boost::optional<double> rather). We should limit the number of times we do std::stod conversions. Relates #3700 .

eg

hpp

  std::string m_extraterrestrialDirectNormalRadiation; //units Wh/m2, missing 9999., minimum 0

cpp

  boost::optional<double> EpwDataPoint::extraterrestrialDirectNormalRadiation() const
  {
    if(m_extraterrestrialDirectNormalRadiation == "9999") {
      return boost::none;
    }
    return boost::optional<double>(std::stod(m_extraterrestrialDirectNormalRadiation));
  }

  bool EpwDataPoint::setExtraterrestrialDirectNormalRadiation(double value)
  {
    if(0 > value || value == 9999) {
      m_extraterrestrialDirectNormalRadiation = "9999";
      return false;
    }
    m_extraterrestrialDirectNormalRadiation = std::to_string(value);
    return true;
  }
@jmarrec jmarrec changed the title revamp EpwDataPoint to use double attributes when appropriate revamp EpwDataPoint to use double attributes where appropriate Oct 1, 2019
@jasondegraw
Copy link
Member

jasondegraw commented Jan 14, 2020

While there's always room for improvement, using strings for storage was chosen to avoid the (inevitable) loss of precision inherent in string-to-double-to-string conversions, which is why this part of the EPW was added in the first place. Undoing that would be a step backward.

@tanushree04 tanushree04 added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Jul 21, 2020
@tijcolem tijcolem removed the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants