Conversation
mnabideltares
left a comment
There was a problem hiding this comment.
@WPK4FEM
Thank you for this nice work.
Generally looks good to me, however I have minor comments.
|
|
||
| # assert if average error in all stages is below accuracy | ||
| accuracy = 1.0e-7 | ||
| accuracy = 1.0e-3 |
There was a problem hiding this comment.
The accuracy is decreased here. It may happen that the tests are still not converging, but it hits thir criterium?
There was a problem hiding this comment.
Checked that they are converging.
| @@ -0,0 +1,179 @@ | |||
| { | |||
| "problem_data": { | |||
| "problem_name": "mesh", | |||
There was a problem hiding this comment.
Problem_name preferly to describe the type of problem. Setting it as "mesh" is not descriptive.
There was a problem hiding this comment.
Changed to "rising_falling_phreatic_level"
| "problem_data": { | ||
| "problem_name": "mesh", | ||
| "start_time": 0.0, | ||
| "end_time": 29.0, |
There was a problem hiding this comment.
Is there any specific reason to set it to 29? Why not 30?
There was a problem hiding this comment.
It is designed this way to have regular human recognizable phreatic levels a the chosen time points.
| }, | ||
| "time_stepping": { | ||
| "time_step": 1.0, | ||
| "max_delta_time_factor": 1.0 |
There was a problem hiding this comment.
As increase_factor is set to 1, this "max_delta_time_factor" is not necessary.
| "block_builder": true, | ||
| "solution_type": "Quasi-Static", | ||
| "scheme_type": "Backward_Euler", | ||
| "reset_displacements": true, |
There was a problem hiding this comment.
This is a single stage calculation. Do we still need reset_displacement?
There was a problem hiding this comment.
No, strictly speaking not. Removed the setting.
| "model_part_name": "PorousDomain.TopWaterPressure", | ||
| "variable_name": "WATER_PRESSURE", | ||
| "is_fixed": true, | ||
| "table": [1,1], |
There was a problem hiding this comment.
A bit confused here. The Pressure is scalar, but "table": [1,1] is used, while I am expecting "table":1. Or maybe "Phreatic_Multi_Line" requires this?
There was a problem hiding this comment.
There are 2 points that define the Phreatic_Multi_line here. The tables give the change in head for each point in time.
| "model_part_name": "PorousDomain.BottomWaterPressure", | ||
| "variable_name": "WATER_PRESSURE", | ||
| "is_fixed": true, | ||
| "table": [1,1], |
There was a problem hiding this comment.
Same as above, "table": 1?
There was a problem hiding this comment.
see previous response.
| "model_part_name": "PorousDomain.InitialWaterPressure", | ||
| "variable_name": "WATER_PRESSURE", | ||
| "is_fixed": false, | ||
| "table": [0,0], |
There was a problem hiding this comment.
see previous response
|
|
||
| Begin SubModelPart BottomWaterPressure | ||
| Begin SubModelPartTables | ||
| 1 |
There was a problem hiding this comment.
The TopWaterPressure and BottomWaterPressure share the same table. Is it done intentially?
There was a problem hiding this comment.
Yes this is intentional. Boundary conditions are chosen such that the same phreatic level is set on both ends of the mesh.
| "time_stepping": { | ||
| "time_step" : 1.0, | ||
| "time_step" : 0.1, | ||
| "max_delta_time_factor" : 1.0 |
There was a problem hiding this comment.
"increase_factor" : 1.0, then "max_delta_time_factor" not necessary
There was a problem hiding this comment.
removed the setting here and all other project parameters files within test_partial_saturation
mnabideltares
left a comment
There was a problem hiding this comment.
@WPK4FEM
Thank you for changes. It looks OK to me
📝 Description
Changes to partially saturated tests.
🆕 Changelog