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

Introduce DynamicBlockVector #74

Merged
merged 5 commits into from
Apr 10, 2022
Merged

Introduce DynamicBlockVector #74

merged 5 commits into from
Apr 10, 2022

Conversation

peterrum
Copy link
Collaborator

@peterrum peterrum commented Apr 9, 2022

This PR introduces a new block vector type. The motivation is that I have the feeling that the normal block vector class from deal.II is not designed for adaptability changing number of blocks, requiring annoying additional pre- and post-processing steps.

This PR only adds the new vector and does not simplify the usage yet, which will be done in a follow-up PR.

@vovannikov Could you try this PR out. It depends on dealii/dealii#13606 and dealii/dealii#13605.

@vovannikov
Copy link
Collaborator

@peterrum sure! I will try this out today

@vovannikov
Copy link
Collaborator

Tried it, looks good to me.

Should we merge it right now or wait till those related PRs get merged to deal.ii master branch first?

@vovannikov vovannikov self-requested a review April 10, 2022 17:42
@peterrum
Copy link
Collaborator Author

I am fine with merging. Hopefully, the PRs are not too controversial and are merged in the next days!

@peterrum peterrum merged commit 1fbde1a into master Apr 10, 2022
@peterrum
Copy link
Collaborator Author

@vovannikov How do you run your application. Do I need to make some adjustments to the parameters so that I test all features?

@vovannikov
Copy link
Collaborator

@vovannikov How do you run your application. Do I need to make some adjustments to the parameters so that I test all features?

Do you mean functionality not related to this PR? If so, basically I do this couple of runs:

mpirun -np 2 ./applications/sintering/sintering-circle-2D 4
mpirun -np 2 ./applications/sintering/sintering-cloud-2D ../pf-applications/applications/sintering/sintering_cloud_examples/5particles.cloud

It won't still test the non-greedy mode of the Grain Tracker but that feature will be removed shortly.

@peterrum
Copy link
Collaborator Author

If I run the second test with 40 processes, I get:

    An error occurred in line <102> of file
    </home/munch/sw-index/pf-applications/include/pf-applications/grain_tracker/tracker.h>
    in function
    std::tuple<bool, bool> GrainTracker::Tracker<dim, Number>::track(const
    BlockVectorType&) [with int dim = 2; Number = double;
    GrainTracker::Tracker<dim, Number>::BlockVectorType =
    dealii::LinearAlgebra::distributed::DynamicBlockVector<double>]
    The violated condition was:
    old_grains.at(grain_index_at_min_distance).get_order_parameter_id() ==
    cloud.get_order_parameter_id()
    Additional information:
    Something got wrong with the order parameters numbering:
    
    grain_index_at_min_distance = 2
    old grain order parameter   = 1
    cloud order parameter       = 0
    min_distance                = -18.250017
    
    Stacktrace:
    -----------
    #0  ./applications/sintering/sintering-cloud-2D:
    GrainTracker::Tracker<2,
    double>::track(dealii::LinearAlgebra::distributed::DynamicBlockVector<double>
    const&)
    #1  ./applications/sintering/sintering-cloud-2D: Sintering::Problem<2,
    double, dealii::VectorizedArray<double, 8ul> >::run()::{lambda(double,
    bool)#7}::operator()(double, bool) const
    #2  ./applications/sintering/sintering-cloud-2D: Sintering::Problem<2,
    double, dealii::VectorizedArray<double, 8ul> >::run()
    #3  ./applications/sintering/sintering-cloud-2D: main

That does not look right. If I run with 2 processes, the error message does not come.

@vovannikov
Copy link
Collaborator

Thank you for the report! I've got an idea why it happens, will work on it today.

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

2 participants