Skip to content
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

Read in nonuniform grid #1115

Merged

Conversation

anne-glerum
Copy link
Contributor

This way we can also read in data that is not equally spaced in a direction, for example crust1.0.

@@ -332,12 +332,12 @@ namespace aspect
/**
* Interpolation functions to access the data.
*/
std::vector<Functions::InterpolatedUniformGridData<dim> *> data;
std::vector<Functions::InterpolatedTensorProductGridData<dim> *> data;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you instead just make it a std::vector<Function<dim>*>, for reasons that will become clear below?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note: Function<dim> is a base class for both the uniform and tensor product classes.)

@anne-glerum anne-glerum force-pushed the uniform_to_nonuniform_asciidata branch from 95fad9e to 5533e5d Compare July 25, 2016 08:24
@gassmoeller
Copy link
Member

/run-tests

coordinate_values[i].push_back(temp_coord);

// The grid spacing
double first_grid_spacing = 0, grid_spacing = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually split these declarations into two lines, and I do not think you need to initialize these variables (this is my mandatory comment, which proofs that I have looked through the code 😄).

@gassmoeller
Copy link
Member

Very nice and useful addition. I assume you have tested the result? Could you maybe take one of the ascii_data tests and modify them to use a non-uniform grid? One test and one changed file should be enough.

@anne-glerum
Copy link
Contributor Author

@gassmoeller I've incorporated your comments and added a test. I've tested the result by changing existing test data files, now I'll just do one more test with a data file I've generated. I'll let you know :)

@anne-glerum anne-glerum force-pushed the uniform_to_nonuniform_asciidata branch from e585465 to 87d9505 Compare August 4, 2016 12:52
@anne-glerum
Copy link
Contributor Author

@gassmoeller I've changed the tolerance used for comparing the grid spacing and added output when the data is found to be nonuniform.

std::numeric_limits<double>::epsilon()*std::abs(grid_spacing+first_grid_spacing)*2)
// Compare current grid spacing with first grid spacing,
// taking into account roundoff of the read-in coordinates
if (grid_spacing / first_grid_spacing < 0.995 || grid_spacing / first_grid_spacing > 1.005)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the previous version of this check was more intuitive. Is your current one not equivalent to replacing std::numeric_limits<double>::epsilon() by 0.005? I would be favouring that if you need a larger tolerance.

@gassmoeller
Copy link
Member

Thanks! Apart from my one stylistic comment this is ready to go. Care to add an entry in changes.h about the new possibility?

Remove unnecessary functions and add assert

Let file format dictate uniform or nonuniform GridData

Improve comments

Indent
Add test

Use a different magic number

Add output and increase tolerance

Update test and reduce refinement

Astyle
@anne-glerum anne-glerum force-pushed the uniform_to_nonuniform_asciidata branch from 87d9505 to a80efdf Compare August 5, 2016 15:01
@gassmoeller gassmoeller merged commit 6437fc0 into geodynamics:master Aug 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants