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

Level set: composition #14

Merged
merged 78 commits into from
Dec 10, 2020
Merged

Conversation

peterrum
Copy link
Collaborator

@peterrum peterrum commented Dec 6, 2020

This PR restructures the level-set module so that reinitialization, advection, curvature_computation, and normal_vector_computation become operators on their own and the level_set operator uses them via composition.

FYI @mschreter The dependencies between the modules still need to be made more clear but this should allow us to use them from extern.

@peterrum peterrum marked this pull request as draft December 6, 2020 21:25
Copy link
Collaborator Author

@peterrum peterrum left a comment

Choose a reason for hiding this comment

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

Comment on lines 70 to 74

/**
* TODO
*/
TimeStepping::Scheme time_step_scheme;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed.

@@ -262,6 +262,7 @@ LevelSetOKZSolver<dim>::initialize_data_structures()
params.convection_stabilization = this->parameters.convection_stabilization;
params.do_iteration = this->parameters.do_iteration;
params.tol_nl_iteration = this->parameters.tol_nl_iteration;
params.time_step_scheme = this->parameters.time_step_scheme;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed.

@@ -113,9 +113,8 @@ class LevelSetOKZSolverAdvanceConcentration
const LevelSetOKZSolverAdvanceConcentrationBoundaryDescriptor<dim> &boundary,
const MatrixFree<dim> & matrix_free,
const LevelSetOKZSolverAdvanceConcentrationParameter & parameters,
double & global_max_velocity,
const DiagonalPreconditioner<double> & preconditioner,
AlignedVector<Tensor<1, dim, VectorizedArray<double>>> &evaluated_convection);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mschreter Please not the updated interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice.

Comment on lines 27 to 33
AssertThrow(degree_u >= 2 && degree_u <= 5, ExcNotImplemented()); \
AssertThrow(degree_u >= 1 && degree_u <= 5, ExcNotImplemented()); \
AssertThrow(ls_degree >= 1 && ls_degree <= 4, ExcNotImplemented()); \
if (ls_degree == 1) \
{ \
if (degree_u == 2) \
if (degree_u == 1) \
OPERATION(1, 1); \
else if (degree_u == 2) \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/**
* TODO: needed? this is equivalent to `fe.tensor_degree()+1`?
*/
unsigned int concentration_subdivisions;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI @mschreter :)

@peterrum peterrum marked this pull request as ready for review December 10, 2020 10:33
@peterrum peterrum changed the title [WIP] Level set: composition Level set: composition Dec 10, 2020
@peterrum
Copy link
Collaborator Author

@kronbichler This PR contains the hard work of two very intensive days by @mschreter and myself, trying to to use more modules of adaflo in MeltPoolDG. The work here is definitely no finished yet: 1) there are many TODOs, which is mainly related to the fact that we have not had the time to document everything to a level as we wanted and 2) we have only had time to integrate the advection operator till now, so that the interfaces, as proposed here, might still change. Nevertheless, I think it would be a good idea to merge so that I can open the follow-up PR for the simplex support and other follow-up PRs have a more human size. What do you think? I have run all tests locally: all are still passing.

Comment on lines +210 to +211
// [PM]: The following code has been removed since it was not used in any test
// and it introduced annoying cyclic dependencies.
Copy link
Owner

Choose a reason for hiding this comment

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

This is experimental code anyway, so it can remain in this state until there appears a need for it again with some additional experiments.

@kronbichler kronbichler merged commit 22e40d1 into kronbichler:master Dec 10, 2020
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.

3 participants