Conversation
kylebeggs
left a comment
There was a problem hiding this comment.
overall good start. I think what we really need here for the is the Holy traits system (named after a programmer/prof Tim Holy). an example being that adiabatic has the 'trait" of being a dirichlet bc. we can then use this trait to dispatch cleanly to make_bc. read up on this and lets discuss. I think you were starting to get to this design pattern already without knowing it! i am a big fan of it.
kylebeggs
left a comment
There was a problem hiding this comment.
Overall a good step forward, but i had some thoughts and used Claude to help me write them down in a readable manner form my bullet point thoughts. I then had it implement it. I'll make a PR if you like it:
The Problem
The original BC system had separate types for each physics domain:
struct Temperature{T} <: Dirichlet ... end # energy
struct Adiabatic <: Neumann ... end # energy
struct VelocityInlet{T} <: Dirichlet ... end # fluidsThis creates two issues:
-
Code duplication: An adiabatic wall (
dT/dn = 0) and a zero-gradient velocity outlet (dv/dn = 0) are mathematically identical Neumann conditions. Both need the samewrite_bc_neumann!treatment, but we'd need separate types and potentially duplicate code paths. -
No physics validation: Nothing prevents using
TemperaturewithIncompressibleNavierStokes. The code silently accepts it.
The Solution: Two Orthogonal Dimensions
We separate two concerns that were conflated:
| Dimension | Purpose | Examples |
|---|---|---|
| Mathematical type | How the BC modifies the linear system | Dirichlet, Neumann, Robin |
| Physics domain | Which models this BC applies to | Energy, Fluids, Walls |
Mathematical Hierarchy (Existing)
This was already correct and unchanged:
AbstractBoundaryCondition
├── Dirichlet → write_bc_dirichlet!
└── DerivativeBoundaryCondition
├── Neumann → write_bc_neumann!
└── Robin → write_bc_robin!
Physics Domain (New)
Added as a type parameter:
struct FixedValue{P <: PhysicsDomain, T} <: Dirichlet
value::T
end
struct ZeroGradient{P <: PhysicsDomain} <: Neumann endNow ZeroGradient{EnergyPhysics} and ZeroGradient{FluidPhysics} share the same implementation but are distinguishable for validation.
Why Type Aliases, Not Separate Types?
We use const aliases to preserve user-friendly names:
const Temperature{T} = FixedValue{EnergyPhysics, T}
const Adiabatic = ZeroGradient{EnergyPhysics}
const VelocityOutlet = ZeroGradient{FluidPhysics}Benefits:
Temperature(100.0)still works (unchanged API)bc isa Temperaturestill works (type checking)bc isa FixedValuealso works (generic checks)- Single implementation for mathematically equivalent BCs
Why Runtime Validation, Not Compile-Time?
We considered parametric constraints on Domain{P}, but:
- Domain stores BCs in a
Dict(heterogeneous collection) - Models are stored as
AbstractModelvector - Full compile-time enforcement would require complex type gymnastics
Runtime validation in the Domain constructor is simpler and gives better error messages:
# In Domain constructor:
for (surf_name, bc) in boundaries
if !is_compatible(physics_domain(typeof(bc)), physics_domain(typeof(model)))
throw(ArgumentError("BC ... incompatible with model ..."))
end
endThe Physics Trait System
Uses Holy traits pattern (zero runtime cost):
abstract type PhysicsDomain end
struct EnergyPhysics <: PhysicsDomain end
struct FluidPhysics <: PhysicsDomain end
struct WallPhysics <: PhysicsDomain end # Compatible with both
struct GenericPhysics <: PhysicsDomain end # Default, skips validation
# Trait accessor
physics_domain(::Type{<:Temperature}) = EnergyPhysics()
physics_domain(::Type{<:SolidEnergy}) = EnergyPhysics()
# Compatibility
is_compatible(::EnergyPhysics, ::EnergyPhysics) = true
is_compatible(::WallPhysics, ::FluidPhysics) = true # Walls work with fluids
is_compatible(::EnergyPhysics, ::FluidPhysics) = falseWhat About Physics-Specific BCs?
Some BCs have semantic meaning beyond their math:
Convectionhash,k,T_inf(heat transfer coefficient, conductivity, ambient temp)PressureOutlettargets the pressure field specifically, not velocity
These remain as standalone types with physics_domain traits:
struct Convection{H, K, T} <: Robin
h::H # semantic field names
k::K
T_inf::T
end
physics_domain(::Type{<:Convection}) = EnergyPhysics()Summary
| Before | After |
|---|---|
| Separate types per physics | Parametric types BC{PhysicsDomain} |
| No validation | Runtime validation in Domain |
| Duplicate math implementations | Shared via type parameters |
| N/A | New VelocityOutlet for fluids |
The key insight: separate what varies (physics domain) from what's shared (mathematical treatment).
|
I have fixed the issues with the normals and made boundary condition enforcement faster by avoiding large allocations, did not implement your suggestions on the specification of the physical model yet, and also still need to implement the interface for the time-varying boundary conditions |
| is_compatible(::T, ::T) where {T<:PhysicsDomain} = true | ||
| is_compatible(::WallPhysics, ::FluidPhysics) = true | ||
| is_compatible(::WallPhysics, ::EnergyPhysics) = true | ||
| is_compatible(::PhysicsDomain, ::PhysicsDomain) = false |
There was a problem hiding this comment.
i guess this is just the fallback that if not defined, we make it false?
There was a problem hiding this comment.
Made it true with a warning:
#same domain
is_compatible(::T, ::T) where {T <: PhysicsDomain} = true
#different models with WallPhysics
is_compatible(::WallPhysics, ::FluidPhysics) = true
is_compatible(::WallPhysics, ::EnergyPhysics) = true
#Fallback rule: different domains are compatible but give warning
function is_compatible(bc::PhysicsDomain, model::PhysicsDomain)
@warn """
UNDEFINED PHYSICS DOMAIN COMPATIBILITY:
BC domain: $(typeof(bc))
Model domain: $(typeof(model))
There is no compatibility rule defined for these domains.
This BC will be allowed, but consider defining explicit compatibility rules in `physics_traits.jl`.
"""
return true
end
|
I finished addressing your observations and wrote a first test for solution of 2d heat equation with method of manufactured solutions. Now we just need to think about the unsteady case and then we are done with BCs I think |
kylebeggs
left a comment
There was a problem hiding this comment.
generally looks good to me, i think once tests are passing we can merge. Julia 1.10 tests are not passing currently
|
@Davide-Miotti there are issues with the new WHatsThePoint, etc. honestly since this is still an un-regstered and provate repo, Im going to just merge and then when we get WhatsThePoint merged we can come back to this. It will just be easier. The tests pass locally. |
The goal is to make boundary conditions simple to understand and implement for any type of problem. Everything is still to be tested and of course a lot of code optimization still to do