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

Update GroundHeatExchangerHorizontalTrench: wrap Site:GroundTemperature:Undisturbed:XXX objects #4734

Closed
jmarrec opened this issue Oct 31, 2022 · 0 comments · Fixed by #4717
Assignees
Labels
APIChange A motivated non-backward compatible change that breaks the existing API and needs to be communicated component - HVAC component - Version Translator Enhancement Request

Comments

@jmarrec
Copy link
Collaborator

jmarrec commented Oct 31, 2022

Enhancement Request

Following #4716 , the GroundHeatExchangerHorizontalTrench is outdated:

  • The Ground Temperature Model field accepts ·SiteGroundTemperature but that isn't even functional
  • The only (currently broken in FT) option is Site:GroundTemperature:Undisturbed:KusudaAchenbach, the Site:GroundTemperature:Undisturbed:Xing and Site:GroundTemperature:Undisturbed:FiniteDifference aren't possible options

Detailed Description

Possible Implementation

Option B proposed by @joseph-robertson on #4717 (comment)

Wrap Site:GroundTemperature:Undisturbed:KusudaAchenbach for starters (FiniteDifference and Xing could be done later)

Deprecate the getters / setters (with a clear message saying it'll be removed in 3.8.0 or something).

double kusudaAchenbachAverageSurfaceTemperature() const;
double kusudaAchenbachAverageAmplitudeofSurfaceTemperature() const;
double kusudaAchenbachPhaseShiftofMinimumSurfaceTemperature() const;

If we were to wrap the Xing/FiniteDifference, these would just throw if it's not Kusuda (pseudo-code):

OS_DEPRECATED double GroundHeatExchangerHorizontalTrench::kusudaAchenbachAverageSurfaceTemperature() {
  LOG(Warn, "GroundHeatExchangerHorizontalTrench::kusudaAchenbachAverageSurfaceTemperature is deprecated and will be removed in 3.8.0. Please query the groundModel object.");
  if (groundModel().IddObjectType() != "Kusuda") {
    LOG_AND_THROW("...")
  } 

  return groundModel().cast<Kusuda>().averageSurfaceTemperature();
}

I agree with Joe: Existing Ctor would create a KusudaAchenbach for you. Make a second ctor where you pass in the ground model object.

Requires VT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIChange A motivated non-backward compatible change that breaks the existing API and needs to be communicated component - HVAC component - Version Translator Enhancement Request
Projects
None yet
2 participants