Skip to content

Conversation

damiano-flex
Copy link
Contributor

@damiano-flex damiano-flex commented Jul 24, 2025

Greptile Summary

This PR introduces a new NaturalConvectionVerticalSpec class to enhance thermal boundary condition modeling in the Tidy3D framework. The class enables automatic calculation of heat transfer coefficients for natural convection from vertical plates based on fluid properties rather than requiring manual coefficient specification.

The implementation follows standard heat transfer engineering practices by using Rayleigh number calculations and appropriate Nusselt number correlations to determine heat transfer coefficients. It supports both laminar and turbulent flow regimes based on critical Rayleigh number thresholds. The class accepts comprehensive fluid property inputs including thermal conductivity, viscosity, density, specific heat, and thermal expansion coefficient.

The integration is achieved by extending the existing ConvectionBC.transfer_coeff field to accept either a simple float (existing behavior) or the new NaturalConvectionVerticalSpec object, maintaining backward compatibility. The class is properly exported through the main package interface and integrated into the TCAD types system, following established patterns in the codebase for thermal boundary conditions.

Confidence score: 2/5

  • This PR has significant issues that could cause problems if merged, particularly around unit consistency and incomplete implementation
  • The unit system mismatch between SI units and Tidy3D's micrometer-based system could lead to incorrect calculations, and the _compute_parameters method returns unused values suggesting incomplete integration
  • Files tidy3d/components/tcad/boundary/heat.py needs immediate attention for unit consistency, parameter validation, and completion of the computation logic

@damiano-flex damiano-flex requested a review from marc-flex July 24, 2025 15:54
greptile-apps[bot]

This comment was marked as resolved.

@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from bbe0d31 to f737cae Compare July 24, 2025 16:00

This comment was marked as outdated.

@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from f737cae to bbe0d31 Compare July 28, 2025 17:14
@damiano-flex damiano-flex changed the title [FXC-1512]: [DRAFT]: Add natural convection BC [FXC-1512]: Add natural convection BC Jul 30, 2025
marc-flex

This comment was marked as resolved.

dbochkov-flexcompute

This comment was marked as resolved.

@dbochkov-flexcompute

This comment was marked as resolved.

marc-flex

This comment was marked as resolved.

dbochkov-flexcompute

This comment was marked as resolved.

@damiano-flex damiano-flex changed the title [FXC-1512]: Add natural convection BC feat: Add natural convection BC Aug 21, 2025
@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from 2e9f493 to e48989a Compare August 21, 2025 16:40
dbochkov-flexcompute

This comment was marked as resolved.

@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from e48989a to c6b941e Compare August 21, 2025 21:25
@damiano-flex damiano-flex enabled auto-merge August 21, 2025 21:25
@damiano-flex

This comment was marked as resolved.

@damiano-flex damiano-flex disabled auto-merge August 21, 2025 21:35
@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from c6b941e to 25cd6b2 Compare August 21, 2025 21:36
@damiano-flex damiano-flex enabled auto-merge August 21, 2025 21:39
@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch 2 times, most recently from a424309 to 8e8cf4f Compare August 21, 2025 21:53
@damiano-flex damiano-flex force-pushed the damiano/natural_convection_BC branch from 8e8cf4f to fbf5747 Compare August 21, 2025 22:01
@damiano-flex damiano-flex disabled auto-merge August 22, 2025 07:33
@damiano-flex damiano-flex enabled auto-merge August 22, 2025 07:33
@damiano-flex damiano-flex added this pull request to the merge queue Aug 22, 2025
Merged via the queue into develop with commit 03c82be Aug 22, 2025
24 checks passed
@damiano-flex damiano-flex deleted the damiano/natural_convection_BC branch August 22, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants