adamantine-sim / adamantine

Software to simulate heat transfer for additive manufacturing
https://adamantine-sim.github.io/adamantine/
Other
36 stars 10 forks source link

Add missing BC inputs in the 316 demo input file #134

Closed stvdwtt closed 2 years ago

stvdwtt commented 2 years ago

The demo_316_short.info input file was missing the entries necessary for convective and radiative BCs and therefore the calculation went to all NANs.

Currently we don't have any notification if the user selects convective or radiative BCs but doesn't give the expected input parameters for those BC models. Part of this is how things are currently split up -- ThermalOperator handles the BCs but doesn't have access to the input material database and MaterialProperty handles the material properties but doesn't have access to the BC type. One option would be to have a check in ThermalPhysics to see if things are appropriately defined (ThermalPhysics currently gets the whole input database). Another option would be to have something in MaterialProperty that tracks what is defined and what isn't, and then ThermalOperator can check to see if the appropriate parameters are defined based on the BC choice. @Rombur, do you have a preference on how to handle this?

Rombur commented 2 years ago

We have the same problem with other parameters like thermal_conductivity. If you forget or misspell one of the 3 parameters (x, y, z) the code will crash without a warning. I think that we should check that the input file is correct right after we read it in adamantine.cc. The only reason I haven't done before is because it means that we need to duplicate the name of the properties. The names are defined here https://github.com/adamantine-sim/adamantine/blob/master/source/MaterialProperty.templates.hh#L640-L643 but also there https://github.com/adamantine-sim/adamantine/blob/master/source/types.hh#L40-L50 and now at a third place. I am just worry of keeping everything synchronized. Maybe if we move the names to types we could avoid too many duplications.

stvdwtt commented 2 years ago

Good idea, I can work on that for a new PR.

Rombur commented 2 years ago

Good idea, I can work on that for a new PR.

Sounds good. I think that the function should be called in adamantine.cc but you should put it the library so that we can call it in the tests too. I definitely wasted a bunch of time because the database I created for a test was wrong.