-
Notifications
You must be signed in to change notification settings - Fork 251
[WIP] Add coordinate system to particle/property/function #6544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[WIP] Add coordinate system to particle/property/function #6544
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this. There are a few issues with the PR as you already noticed. I left comments for how to fix them. Afterwards the test will run without issues (I tested that locally and it works).
After fixing my comments please:
- add a changelog entry
- update the test results
- make sure you run
make indent
Then this should make a nice addition.
|
||
for (unsigned int i = 0; i < n_components; ++i) | ||
data.push_back(function->value(position, i)); | ||
data.push_back(function->value(Utilities::convert_array_to_point<dim>(point.get_coordinates()), i)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is correct, but a bit hard to read. what about:
data.push_back(function->value(Utilities::convert_array_to_point<dim>(point.get_coordinates()), i)); | |
{ | |
const double function_value = function->value(Utilities::convert_array_to_point<dim>(point.get_coordinates()), i) | |
data.push_back(function_value); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this file, it is one of the Paraview files we do not usually include in test output. the same for all the other paraview output files, replace them with the gnuplot output files.
|
||
subsection Particles | ||
set Time between data output = 70 | ||
set Data output format = vtu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you set this to gnuplot
and include the created gnuplot data files instead of the Paraview files? The benefit is that gnuplot files are text files and it is therefore easier to see how the output changes (compared to the binary files of Paraview).
* The coordinate representation to evaluate the function. Possible | ||
* choices are depth, cartesian and spherical. | ||
*/ | ||
Utilities::Coordinates::CoordinateSystem coordinate_system; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order for this to work you need to add #include <aspect/utilities.h>
at the top of the file. Otherwise CoordinateSystem
is unknown in this file.
std::vector<double> &data) const | ||
{ | ||
// convert the position into the selected coordinate system | ||
const Utilities::NaturalCoordinate<dim> point = this->get_geometry_model().cartesian_to_other_coordinates(position, coordinate_system); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order for this to work you also need to derive the Function
class from SimulatorAccess(and include
#include <aspect/simulator_access.h>in the header file). SimulatorAccess is the class that provides the
get_geometry_model` function. You can see how that works for the initial temperature function object here: https://github.com/geodynamics/aspect/blob/main/include/aspect/initial_temperature/function.h#L42
306c097
to
d9307ab
Compare
This PR adds a coordinate system option to
source/particle/property/function.cc
. It follows from the starter pack issue #5626.I attempted to mimic @gassmoeller's examples in #5626, but
is failing. Apparently the parameter
set Coordinate system = spherical
is not declared correctly in, but I cannot figure out why. Here is the relevant part ofsource/particle/property/function.cc
:Here is the relevant part of the prm file
tests/particle_property_multiple_functions_spherical_coordinates.prm
:Here is part of the
ctest
output on my local machine:Lastly, I copy/pasted the test files from
tests/particle_property_multiple_functions
, so you can ignore those until the test actually runs.