Geometry class #147

AliHaider93 · 2024-07-05T14:47:58Z

A class to incapsulate the geometry of LS-DYNA part.

drossberg · 2024-07-06T14:09:49Z

src/conv/k-g/k_parser.h

@@ -25,12 +25,14 @@

 #ifndef KPARSER_INCLUDED
 #define KPARSER_INCLUDED
+#define FIRST_NODE 2


? Where is it used?

You see that you are getting into trouble with the primitives names. How about not having a setName() but setBaseName() and not getName() but getNames() which returns a list of names? In case of the Bot, it would be the base name with a ".bot" attached. In case of the Arbs, it could be a list of base-name + "." + number + ".arb".

? Where is it used?

it's not used any more, I was trying to make a loop and I defined it. I will rimove it. Edit: it is used in k_parser.cpp to fill the nodes in the vector using a loop

I can just use i_n+2 because it's a fixed format, but this seamed nicer. what do you think?

You see that you are getting into trouble with the primitives names. How about not having a setName() but setBaseName() and not getName() but getNames() which returns a list of names? In case of the Bot, it would be the base name with a ".bot" attached. In case of the Arbs, it could be a list of base-name + "." + number + ".arb".

okay, this seems to be more coherent, I am working on it.

the only problem with setBaseName() is in the case of arbs the elements numbers doesn't have to be consequent starting from (1 to n). so if we want to preserve the number of Hex element we have to keep naming the arbs following the .k file. I will update the pull request and then you can tell me what you think

I updated with getNames() and setBaseName(), I will change also Arbs::write() and Bot::write().

? Where is it used?

it's not used any more, I was trying to make a loop and I defined it. I will remove it. Edit: it is used in k_parser.cpp to fill the nodes in the vector using a loop

I can just use i_n+2 because it's a fixed format, but this seamed nicer. what do you think?

Okay, that's a reason. However, in this case, it should be kept in k_parser.cpp (declaration/definition) and a const size_t, e.g.

const size_t FirstNode = 2;

…) so I don't name the arbs twice

…h will cause memory access error

drossberg · 2024-07-15T19:20:05Z

src/conv/k-g/arbs.cpp

    for (std::map<std::string, rt_arb_internal>::iterator it = arbs.begin(); it != arbs.end(); it++) {
+	std::string arbName = name;
+	arbName += it->first;


Maybe, putting a point "." between the base name and the index?

yes it's better , I will update

I just have a question about the constructors and destructors, first the arb constructor

is RT_ARBN_INTERNAL_MAGIC the one to go even with arb4, arb5, arb6, arb7?
and for Geomtry I think it doesn't need a constructor or a destructor or am I wrong?

then I will handle the other arbs and I can clean the code

You need the RT_ARB_INTERNAL_MAGIC here. This is for the rt_arb_internal.

arbn is very different from the arb4 ... arb8.

For the Geometry class, the standard constructor and destructor should do.

drossberg · 2024-07-22T17:04:40Z

src/conv/k-g/k-g.cpp

+
+				std::string arbNumber = std::to_string(*itr);
+
+				geometry.addArb(arbNumber.c_str(), point1, point2, point3, point4, point5, point6, point7, point8);


That's a lot of duplicated code. How about writing a helper function AddArb(), which gets n1, ..., n8 as parameters?

you are right, working on it.

I also relized that I made a mistake in the condition for arb4, when the nodes.size()==4 it is also the shell element that we devided into two triangles

drossberg · 2024-07-22T17:06:37Z

src/conv/k-g/arbs.cpp

+    VSET(temp.pt[4], point5[0], point5[1], point5[2]);
+    VSET(temp.pt[5], point6[0], point6[1], point6[2]);
+    VSET(temp.pt[6], point7[0], point7[1], point7[2]);
+    VSET(temp.pt[7], point8[0], point8[1], point8[2]);


Would it be possible, to use VMOVE() here?

yes it's possible, I updated the code

I also wrote the function AddArb, let me know what you think about it. for arb4 I will resolve it later, I am trying to undrstand the logic in LS-DYNA Solid elment.

drossberg · 2024-07-24T17:15:39Z

src/conv/k-g/k-g.cpp

 			    }
-			    else {
-				bot.addTriangle(point1, point2, point3);
-				bot.addTriangle(point1, point3, point4);


Is here an issue with a closing bracket?

No, I don't think so, I have tested the code with a solid element and shell element. I will retest now to see if there is something wrong

I just tested it, no problems.

drossberg · 2024-07-25T16:34:42Z

src/conv/k-g/region_list.cpp

+	    mk_addmember(names[i].c_str(), &(geometry_head.l), NULL, WMOP_UNION);
+	    mk_addmember(names[i].c_str(), &(all_head.l), NULL, WMOP_UNION);
+	}
+	mk_lfcomb(wdbp, geometry.getBaseName(), &geometry_head, 0);

 	if (region_attributes.size() > 0) {
 	    writeAttributes(wdbp, region_name.c_str(), region_attributes);
 	}

 	mk_addmember(region_name.c_str(), &(all_head.l), NULL, WMOP_UNION);



There is a tab in this line, which should not be there.

drossberg · 2024-07-25T16:39:23Z

src/conv/k-g/arbs.h

+    void        setName(const char* value);
+
+    void        addArb(
+			const char*    arbName,


Is this line break intentionally? (similar in arbs.cpp)

drossberg · 2024-07-25T17:58:43Z

src/conv/k-g/region_list.cpp


-	mk_addmember(bot.getName(), &(bot_head.l), NULL, WMOP_UNION);
-	mk_lfcomb(wdbp, region_name.c_str(), &bot_head, 0);
+	for (int i = 0; i < names.size(); i++) {


I'm getting a compilation error on Debian bookworm here:

/home/rossberg/Devel/BRL-CAD/brlcad.devel_k-g/src/conv/k-g/region_list.cpp: In member function ‘void RegionList::create(rt_wdb*)’: /home/rossberg/Devel/BRL-CAD/brlcad.devel_k-g/src/conv/k-g/region_list.cpp:111:27: error: comparison of integer expressions of different signedness: ‘int’ and ‘std::vector<std::__cxx11::basic_string<char> >::size_type’ {aka ‘long unsigned int’} [-Werror=sign-compare] 111 | for (int i = 0; i < names.size(); i++) { | ~~^~~~~~~~~~~~~~

Changing int to size_t solves this.

drossberg · 2024-07-25T18:01:16Z

src/conv/k-g/region_list.cpp

@@ -140,7 +142,7 @@ Bot& RegionList::getBot
 (
    const std::string& name
 ) {
-    return m_list[name].bot;
+    return m_list[name].geometry.getBot();


I'm getting a compilation error on Debian bookworm here:

/home/rossberg/Devel/BRL-CAD/brlcad.devel_k-g/src/conv/k-g/region_list.cpp: In member function ‘Bot& RegionList::getBot(const std::string&)’: /home/rossberg/Devel/BRL-CAD/brlcad.devel_k-g/src/conv/k-g/region_list.cpp:145:40: error: cannot bind non-const lvalue reference of type ‘Bot&’ to an rvalue of type ‘Bot’ 145 | return m_list[name].geometry.getBot(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~

getBot() doesn't return a reference.

drossberg · 2024-07-25T18:02:35Z

src/conv/k-g/geometry.cpp

+}
+
+
+Bot Geometry::getBot(void) const{


You should change the return type to reference here.

Geomtry class

86e96b0

drossberg reviewed Jul 6, 2024

View reviewed changes

AliHaider93 added 4 commits July 8, 2024 19:35

getNames

514d959

a conseptulization of geomtry.write() still need to modify arb.write(…

e22e89f

…) so I don't name the arbs twice

pssing a refernce to Geomtry so I don't delete the objects twice whic…

71473a5

…h will cause memory access error

updated Names

1f8fad7

drossberg reviewed Jul 15, 2024

View reviewed changes

AliHaider93 added 3 commits July 15, 2024 21:43

dot

2df94d1

cleaning

250d14f

arb4...arb7

8066eef

drossberg reviewed Jul 22, 2024

View reviewed changes

AliHaider93 added 2 commits July 22, 2024 21:00

AddArb

e31eb95

VMOVE

4924032

drossberg reviewed Jul 24, 2024

View reviewed changes

drossberg reviewed Jul 25, 2024

View reviewed changes

small fixes

e35da3b

drossberg reviewed Jul 25, 2024

View reviewed changes

src/conv/k-g/geometry.cpp Outdated

}

Bot Geometry::getBot(void) const{

Copy link

Member

drossberg Jul 25, 2024

Choose a reason for hiding this comment

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

You should change the return type to reference here.

AliHaider93 added 2 commits July 25, 2024 20:19

reference

e3a7761

size_t

3047c28

drossberg merged commit 91dd7c9 into BRL-CAD:devel_k-g Jul 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geometry class #147

Geometry class #147

AliHaider93 commented Jul 5, 2024

drossberg Jul 6, 2024

drossberg Jul 6, 2024

AliHaider93 Jul 7, 2024 •

edited

Loading

AliHaider93 Jul 7, 2024

AliHaider93 Jul 8, 2024

AliHaider93 Jul 8, 2024

drossberg Jul 14, 2024

drossberg Jul 15, 2024

AliHaider93 Jul 15, 2024

AliHaider93 Jul 15, 2024

AliHaider93 Jul 15, 2024

drossberg Jul 21, 2024

drossberg Jul 21, 2024

drossberg Jul 22, 2024

AliHaider93 Jul 22, 2024

AliHaider93 Jul 22, 2024

drossberg Jul 22, 2024

AliHaider93 Jul 22, 2024

AliHaider93 Jul 22, 2024 •

edited

Loading

drossberg Jul 24, 2024

AliHaider93 Jul 24, 2024

AliHaider93 Jul 24, 2024

drossberg Jul 25, 2024

drossberg Jul 25, 2024

drossberg Jul 25, 2024

drossberg Jul 25, 2024

drossberg Jul 25, 2024


		std::string arbNumber = std::to_string(*itr);

		geometry.addArb(arbNumber.c_str(), point1, point2, point3, point4, point5, point6, point7, point8);

Geometry class #147

Geometry class #147

Conversation

AliHaider93 commented Jul 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliHaider93 Jul 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliHaider93 Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AliHaider93 Jul 7, 2024 •

edited

Loading

AliHaider93 Jul 22, 2024 •

edited

Loading