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

BHE_1U flow properties update and bugfixes. #2271

Merged
merged 4 commits into from Nov 28, 2018

Conversation

Projects
None yet
3 participants
@endJunction
Copy link
Member

endJunction commented Nov 27, 2018

Use already extracted code. This prevents code duplication for 2U and the CXA/CXC implementations.

@ChaofanChen Could you please review this?

@endJunction endJunction force-pushed the endJunction:BHE1U_flow_properties branch from 225e530 to a8ba7d9 Nov 27, 2018

@ChaofanChen

This comment has been minimized.

Copy link
Member

ChaofanChen commented Nov 27, 2018

@endJunction I'd love to. Thank you very much.

@endJunction endJunction changed the title BHE_1U flow properties update. BHE_1U flow properties update and bugfixes. Nov 27, 2018

@ChaofanChen

This comment has been minimized.

Copy link
Member

ChaofanChen commented Nov 28, 2018

@endJunction Could you please move the implementation of the constructor and other functions in "CreateFlowAndTemperatureControl.h" to the cpp file? In the case of the CXA/CXC or 2U BHE, we also need to call the function named "createFlowAndTemperatureControl", and this will cause an error of multiple definition when compiling. Besides, we also need to enrich flow and temperature boundary conditions including coupled with heat pump. I am not sure if my understanding is right, but please consider it.

@endJunction

This comment has been minimized.

Copy link
Member Author

endJunction commented Nov 28, 2018

@ChaofanChen CreateFlowAndTemperatureControl.h is not part of this PR. Please open another PR.

double outsideArea() const
{
constexpr double pi = boost::math::constants::pi<double>();
double const d = diameter + wall_thickness;

This comment has been minimized.

@TomFischer

TomFischer Nov 28, 2018

Member

2 * wall_thickness?

@TomFischer
Copy link
Member

TomFischer left a comment

Looks good. After '2 * ...'
When @ChaofanChen has reviewed then the PR can be merged.

@ChaofanChen
Copy link
Member

ChaofanChen left a comment

Looks good.

double outsideArea() const
{
constexpr double pi = boost::math::constants::pi<double>();
double const d = diameter + wall_thickness;

This comment has been minimized.

@ChaofanChen

ChaofanChen Nov 28, 2018

Member

d = diameter + wall_thickness * 2 ?

endJunction and others added some commits Nov 27, 2018

[PL] BHE_1U; Use calculateTMFlowPropsInPipe().
Eliminates code duplication in future 2U implementation.
[PL] BHE; Fix calculateTMFLowPropsAnnulus() impl.
This is used in the CXA/CXC implementations.
[PL] BHE; Fix cross-sect. comp. inlet/outlet pipe.
The result is the same, but semantically it was in wrong order.

@endJunction endJunction force-pushed the endJunction:BHE1U_flow_properties branch from 6973ed7 to 103b07c Nov 28, 2018

@endJunction endJunction merged commit 9d4144f into ufz:master Nov 28, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
deploy/netlify Deploy preview ready!
Details

@endJunction endJunction deleted the endJunction:BHE1U_flow_properties branch Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.