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

[Core] Remove push_back from ModelPart #12903

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
dd02fc8
add additional insert and typecheck to PVS
sunethwarna Dec 5, 2024
2bf1e96
remove push_backs from ModelPart
sunethwarna Dec 5, 2024
2c5d23f
include reduction_utils
sunethwarna Dec 5, 2024
d5521ba
minor
sunethwarna Dec 5, 2024
2f72e0c
func name change
sunethwarna Dec 5, 2024
1e98ea3
fix embedded_skin_utility for Id Change
sunethwarna Dec 10, 2024
9c05b31
allow insertion from const vectors
sunethwarna Dec 10, 2024
abbce15
allow insertion from parent entity ranges
sunethwarna Dec 10, 2024
71ae35f
fix for iterator invalidation
sunethwarna Dec 10, 2024
6d34602
add comments
sunethwarna Dec 10, 2024
f1c4a6f
minor signature modification
sunethwarna Dec 10, 2024
35c1874
fix assign_ms_const_to_neigh_utils
sunethwarna Dec 11, 2024
13e87e7
fix error msg tests
sunethwarna Dec 11, 2024
a50fd2b
fix hdf5
sunethwarna Dec 11, 2024
8a42759
generalize addition from ids
sunethwarna Dec 11, 2024
93293e0
simplify
sunethwarna Dec 11, 2024
6adabf4
fix ranged id addition
sunethwarna Dec 11, 2024
9082f96
Merge remote-tracking branch 'origin/master' into pvs/remove_push_bac…
sunethwarna Dec 12, 2024
d7d4734
fix parmmg
sunethwarna Dec 16, 2024
d718a9a
add benchmarks for modelpart
sunethwarna Dec 16, 2024
4430f5e
minor for comparison
sunethwarna Dec 16, 2024
d7e05de
add type_traits
sunethwarna Dec 18, 2024
fedc6b3
allow rvalue containers in pvs insert
sunethwarna Dec 18, 2024
d642295
allow rvalue containers in model part
sunethwarna Dec 18, 2024
ee29e90
tests for rvalue containers in PVS
sunethwarna Dec 18, 2024
71e3074
more performance improvements
sunethwarna Dec 18, 2024
1024156
fix tests
sunethwarna Dec 18, 2024
a9e8219
minor
sunethwarna Dec 18, 2024
abfdcab
bug fix
sunethwarna Dec 18, 2024
a00437f
fix sub range addition
sunethwarna Jan 2, 2025
36fd864
add a test to subrange addition
sunethwarna Jan 2, 2025
88a5f51
fix test
sunethwarna Jan 2, 2025
22ff3e1
expose shrink_to_fit
sunethwarna Jan 2, 2025
99aca1d
change test
sunethwarna Jan 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions kratos/tests/cpp_tests/sources/test_model_part.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
// Vicente Mataix Ferrandiz
//

// System includes
#include <numeric>

// Project includes
#include "containers/model.h"
#include "testing/testing.h"
Expand Down Expand Up @@ -580,4 +583,74 @@ namespace Kratos::Testing {
model_part.RemoveSubModelPart("Inlet1.sub_inlet.sub_sub_inlet.test"),
"Error: There is no sub model part with name \"sub_sub_inlet\" in model part \"Main.Inlet1.sub_inlet\"");
}

KRATOS_TEST_CASE_IN_SUITE(ModelPartSubRangeAddition, KratosCoreFastSuite)
{
Model model;
auto& r_model_part = model.CreateModelPart("test");
Properties::Pointer p_elem_prop = r_model_part.CreateNewProperties(0);

for (IndexType i = 0; i < 16; ++i) {
r_model_part.CreateNewNode(i + 1, 0.0, 0.0, 0.0);
}

KRATOS_EXPECT_EQ(r_model_part.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_model_part.Nodes().capacity(), 16);
Copy link
Contributor

Choose a reason for hiding this comment

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

No wonder this fails in the CI. Memory management is up to the standard lib's implementation. The only guaranteed way of enforcing a specific capacity is with shrink_to_fit and you haven't called that anywhere.

I'd just remove capacity tests altogether.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have changed to check for the change in capacity and used shrink_to_fit to align memory allocations in different compilers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I don't think that checking shrink_to_fit is necessary but it also doesn't hurt. The other change (checking against an earlier capacity) is better in my opinion.


auto& r_sub_model_part_1 = r_model_part.CreateSubModelPart("sub1");
// following should not do anything hence the capacity should not be changed.
r_sub_model_part_1.AddNodes(r_model_part.Nodes().begin(), r_model_part.Nodes().end());
KRATOS_EXPECT_EQ(r_model_part.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_model_part.Nodes().capacity(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().capacity(), 16);

auto& r_sub_model_part_2 = r_sub_model_part_1.CreateSubModelPart("sub2");
// following should not do anything hence the capacity should not be changed.
r_sub_model_part_2.AddNodes(r_model_part.Nodes().begin() + 8, r_model_part.Nodes().end());
KRATOS_EXPECT_EQ(r_model_part.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_model_part.Nodes().capacity(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().size(), 16);
// the capacity of the r_sub_model_part_1 has increased because, the r_sub_model_part_2
// used the iterators of the r_model_part. Then r_sub_model_part_1 is not a subset of r_model_part
// because even though they are pointing to the same memory locations, the intrusive_ptrs memory locations
// are not a subset.
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().capacity(), 24);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().size(), 8);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().capacity(), 8);

// now we add using a sub range to sub2
auto& r_sub_model_part_3 = r_sub_model_part_1.CreateSubModelPart("sub3");
// following should not do anything hence the capacity should not be changed.
r_sub_model_part_3.AddNodes(r_model_part.Nodes().begin() + 4, r_model_part.Nodes().end());
KRATOS_EXPECT_EQ(r_model_part.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_model_part.Nodes().capacity(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().size(), 16);

// again here the capacity changes because not r_sub_model_part_1 contains intrusive_ptrs
// which are not a sub set of the r_model_part
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().capacity(), 28);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().size(), 8);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().capacity(), 8);
KRATOS_EXPECT_EQ(r_sub_model_part_3.Nodes().size(), 12);
KRATOS_EXPECT_EQ(r_sub_model_part_3.Nodes().capacity(), 12);

auto& r_sub_model_part_4 = r_sub_model_part_3.CreateSubModelPart("sub4");
r_sub_model_part_4.AddNodes(r_sub_model_part_3.Nodes().begin() + 4, r_sub_model_part_3.Nodes().end() - 1);

// now there shouldn't be any change in the size or the capacity, because
// now we added r_sub_model_part_3 items to the r_sub_model_part_4. Then it will add them to the
// r_sub_model_part_4, and when it checks IsSubSet for added nodes with r_sub_model_part_3, then it will
// be a subset, hence all the parent model part additions are ignored.
KRATOS_EXPECT_EQ(r_model_part.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_model_part.Nodes().capacity(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().size(), 16);
KRATOS_EXPECT_EQ(r_sub_model_part_1.Nodes().capacity(), 28);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().size(), 8);
KRATOS_EXPECT_EQ(r_sub_model_part_2.Nodes().capacity(), 8);
KRATOS_EXPECT_EQ(r_sub_model_part_3.Nodes().size(), 12);
KRATOS_EXPECT_EQ(r_sub_model_part_3.Nodes().capacity(), 12);
KRATOS_EXPECT_EQ(r_sub_model_part_4.Nodes().size(), 7);
KRATOS_EXPECT_EQ(r_sub_model_part_4.Nodes().capacity(), 7);
}
} // namespace Kratos::Testing.
Loading