Fix ascii data initial for two merged chunks #5041
Conversation
|
Looks great. I have no suggestions regarding coding style or comments. Is specifying points in Cartesian coordinates the most natural choice for your use case? And is specifying two points (plus the origin) better than specifying the unit normal? Just thoughts; not suggestions :) I'll have another look after the test(s) is(are) added. |
gassmoeller
force-pushed
the
fix_ascii_coordinate_systems
branch
from
ae70003
to
116b2c1
Compare
|
Ok, I think this is ready. I added tests for all the new features (support for initial ascii data for two merged chunks and ellipsoidal chunk and slicing for initial temperature datasets) which also found and fixed one bug from the previous version: ellipsoidal chunk also uses latitude instead of colatitude as natural coordinate, which requires special treatment when looking up the ascii data file. Let me know what you think. |
|
| /** | ||
| * The matrix that describes the rotation by which a 2D model | ||
| * needs to be transformed to a plane that contains the origin and | ||
| * the two prescribed points given in the input. | ||
| */ | ||
| Tensor<2,3> rotation_matrix; |
There was a problem hiding this comment.
This means that the slice has to go through the origin, right? I think this might be worth documenting. (But it shouldn't hold up the patch.)
| // Unfortunately, the chunk model uses latitude as natural coordinate. We need to convert this | ||
| // to colatitude to be consistent with the input files for spherical shells and spheres. | ||
| if (Plugins::plugin_type_matches<const GeometryModel::Chunk<dim>> (this->get_geometry_model()) || | ||
| Plugins::plugin_type_matches<const GeometryModel::TwoMergedChunks<dim>> (this->get_geometry_model()) || | ||
| Plugins::plugin_type_matches<const GeometryModel::EllipsoidalChunk<dim>> (this->get_geometry_model())) | ||
| { | ||
| internal_position[2] = numbers::PI/2. - internal_position[2]; | ||
| } |
This PR improves upon #4490 by moving the slice operation into the AsciiDataInitial class. This specifically improves three aspects:
cartesian_to_natural_coordinatesof the geometry model (because if a slice of a dataset was used the number of dimensions of a point was not equal to the number of dimensions of the model). This limitation is now gone, which in particular meansTwoMergedBoxesandEllipsoidalChunkshould now work withAsciiDataInitial. I will add a test for them to this PR tomorrow.spacedimtemplate that is not used in the rest of ASPECT and makesAsciiDataInitialharder to understand.