[FXC-1512]: Add natural convection BC

[damiano-flex]

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

[greptile-apps]

_{3 files reviewed, 5 comments}

_{Edit Code Review Bot Settings | Greptile}

[greptile-apps]

logic: Method returns computed values but they're not used anywhere. Consider if this method should modify instance state or be called differently.

[marc-flex]

Thanks @damiano-flex ! This is looking very good from the get-go!

Apart from the minor comments that I left we're missing unit tests. I think once you add these we'll be good to go

[marc-flex]

Maybe add in the docstring that one can define the constants in SI with from_si_units and maybe add a couple of examples like with the rest of the BC classes: one using the standard constructor and another one with the from_si_units function?

[damiano-flex]

Yes, it can help readability.

[dbochkov-flexcompute]

overall looks all makes sense, but I would add all required fluid parameters into FluidMedium (probably using more explicit names like viscosity, expansivity, etc, to be consistent with SolidMedium) and make this new convection bc automatically get those parameters from fluids it touches. It means that we also need to add a validator that checks that these pararmeters are defined for fluid mediums in our simulations if natural convection is used.

Additionally, I would also move _compute_parameters to the backend

[dbochkov-flexcompute]

should all fluid properties be pulled automatically from medium properties that touches that boundary? Sounds like we should expand heat_spec with these additional properties. For other parameters I would use more explicit names: plate_L -> plate_length

[damiano-flex]

I made some thoughts before answer this comment. Ultimately, I agree, pulling medium properties seems indeed an appropriate choice. If one wants to add 4 natural convection walls the code would look like much more cleaner.

[dbochkov-flexcompute]

I would add all required fluid parameters into FluidMedium (probably using more explicit names like viscosity, expansivity, etc, to be consistent with SolidMedium) and make this new convection bc automatically get those parameters from fluids it touches. It means that we also need to add a validator that checks that these pararmeters are defined for fluid mediums in our simulations if natural convection is used.

Some more out-loud thinking:

Probably still need to have a medium parameter in the natural convection if one wants to define it on a simulation boundary, something like

h_spec = NaturalConvectionVerticalSpec(medium=None, plate_length=1)  # automatically determine which mediums are touched
h_spec = NaturalConvectionVerticalSpec(medium=oil, plate_length=1)  # use parameters defined in oil

Might also make sense to restrict automatic medium=None to StructureStructureInterface and MediumMediumInterface boundary placements only so that we can easily validate that one medium is solid and the other is fluid with all necessary parameters. And always require medium for other boundary placements

What do you think?