input different coordinate systems in initial temperature function #1628
Conversation
|
Follow-up to #1563. |
|
/run-tests |
| * The coordinate representation to evaluate the function. Possible | ||
| * choices are depth, cartesian and spherical. | ||
| */ | ||
| ::aspect::Utilities::Coordinates::CoordinateSystem coordinate_system; |
There was a problem hiding this comment.
I don't think you actually need the ::aspect:: at the front. Same everywhere else.
There was a problem hiding this comment.
It might be needed to differentiate it from dealii::Utilities?
There was a problem hiding this comment.
It should find aspect::Utilities first when you just say Utilities. Can you try, @mbweller ?
There was a problem hiding this comment.
I tried, and it didn't work. It didn't give a compilation error, it lost functionality. Specifically it didn't recognize set Coordinate system.
There was a problem hiding this comment.
I don't understand this. If it compiles, it must work. Can you post what error message or fault you get?
There was a problem hiding this comment.
I can't explain it. I just recompiled, and it works. Earlier it did not, and the only solution was to add ::aspect:: to Utilities:: ...
| Point<dim> point; | ||
|
|
||
| for (unsigned int i = 0; i<dim; ++i) | ||
| point[i] = spherical_coordinates[i]; |
| return function.value(point); | ||
| } | ||
| else if (coordinate_system == ::aspect::Utilities::Coordinates::CoordinateSystem::depth) | ||
| { |
| @@ -64,6 +111,10 @@ namespace aspect | |||
| prm.enter_subsection ("Initial temperature model"); | |||
| { | |||
| prm.enter_subsection("Function"); | |||
| { | |||
| coordinate_system = ::aspect::Utilities::Coordinates::string_to_coordinate_system(prm.get("Coordinate system")); | |||
|
Addressed small issues. Will address functionality in #1632 following this. |
|
I'd like to use r, theta, phi to input single degree order temperature field in my geoid benchmark prm file when this get merged. |
|
This looks good to me. Do you think it is practical to add tests for each of these places? |
|
Do you mean adding the same test prm file with single degree order perturbation for both geoid and this spherical coordinate initial temperature function? |
|
Ideally, we'd like to have tests that run through every new code path that is added to ASPECT. In this case, that would mean adding tests for each coordinate system for each plugin that you are modifying. That may be a lot of new tests, and I don't know whether that's practical, but it would be nice to have. |
mbweller
force-pushed
the
initial_temperature_coor
branch
from
d10525c
to
47d05f9
Compare
|
Had to make a small change to function.h (broken functionality). It works with test cases. |
There was a problem hiding this comment.
Great, @mbweller , and many thanks for the tests! I marked up the use of ::aspect::Utilities, but let me rather go ahead and merge -- this can be fixed at a later time, and maybe that's appropriate anyway because there are other places already.
| * The coordinate representation to evaluate the function. Possible | ||
| * choices are depth, cartesian and spherical. | ||
| */ | ||
| ::aspect::Utilities::Coordinates::CoordinateSystem coordinate_system; |
There was a problem hiding this comment.
It should find aspect::Utilities first when you just say Utilities. Can you try, @mbweller ?
Follow up to boundary condition changes