KeithSloan / GDML

FreeCAD GDML Workbench - AddonManager Installable
Other
44 stars 15 forks source link

Elliptical Cuts #108

Open Alex2671 opened 1 year ago

Alex2671 commented 1 year ago

Greetings! When i press the cut button, i have following prompt:

[\<SelectionObject>,\<SelectionObject>] Boolean Cut

but nothing happened. What could it be? With part workbench it runs fine meanwhile.

FreeCAD 0.20.2 GDML - last Release

Alex2671 commented 1 year ago

Actually i figure out what is happening: when i cut two objects, one of them are pulled out from cut base property and falling under element tree. Then, when its to be sticked again we just need to recompute object to see it properly.

KeithSloan commented 1 year ago

Is that the cut button in the GDML workbench or the Part workbench?

The Boolean operations in the GDML workbench assume that there is one GDML object per Part/(Volume) and you select the two Parts for the boolean operation.

From you last comment, I will check if there is a missing recompute following the Cut.

KeithSloan commented 1 year ago

Okay checked the code and a GDML boolean operation creates a new Object for which a recompute was performed, but not for the Tool and Base Object that get removed. Have push an update that will now do a Document recompute for GDML booleans. Interested to know if this fixes the issue you are seeing.

Alex2671 commented 1 year ago

test.gdml.zip

And there is one more: as you can see at Bool_Cut object, when i do cuts with y axis translations, it is not saving the object in good manner. Even after editing, saving and reopen it states on the brocken place. May be it also couldn't work with x-translation, i didn't test it.

https://github.com/KeithSloan/GDML/issues/108#issuecomment-1490401375 Hm, interesting behavior!

KeithSloan commented 1 year ago

Okay I am trying to recreate. If I create the two Ellipitical Tubes and Cut I get

AfterCut-worldVOL.gdml.txt BeforeCut.FCStd.txt AfterCut.FCStd.txt

The translation that at issue is and the tube ElTube_007 ?

At what point are you applying the y translation? Is it being applied to the GDML Object ElTube_007 or the Part that contains the ElTube_007 before the Cut?

KeithSloan commented 1 year ago

BeforeCutObjPlacement.FCStd.txt BeforeCutPartPlacement.FCStd.txt

If the translation is applied to the Part before the boolean, for me it seems to be okay. If the translation is applied to the GDMLObject before the boolean, the translation is lost.

Is this what you are also seeing.

KeithSloan commented 1 year ago

Have pushed a fix for Boolean Cut Placement.Bases

Rotations not yet handled and needs to be applied to booleans other than Cut.

Would appreciate if you could test, so that can at least apply to other booleans.

KeithSloan commented 1 year ago

Other booleans Placement Bases fix pushed.

Alex2671 commented 1 year ago

Now it works good with thanslations. But still in bool cut operation base property not filled out. I still should do it by myself.

And one more I found wondering: when you point, for example, dz for box or tube, it means full length along z, but for ELTube it says a half width along z axis. Shouldn't it be full length, since its not a radius?

KeithSloan commented 1 year ago

"Now it works good with thanslations. But still in bool cut operation base property not filled out. I still should do it by myself"

There have been a number of updates to the code, please could you check this is with latest code

This is what I am seeing with test files, one where change of Placement is in Part and the other in GDMLElTube

Base seems to be set to me.

Image 03-04-2023 at 11 59

Image 03-04-2023 at 11 59 BeforeCutObjPlacement.FCStd.txt BeforeCutPartPlacement.FCStd.txt

Manual says

3.4.4 Elliptical Tube The GDML Elliptical Tube is formed using 3 dimensions: dx x semi axis dy y semi axis dz z semi axis

You are correct, what is implemented, z is treated as full length.

Need to check which is correct.

KeithSloan commented 1 year ago

ElTube looks correct in Geant4, so assume there is an error in the manaul and I need to change the description in the GDML ElTube property description.

Image 03-04-2023 at 12 29

Alex2671 commented 1 year ago

I do a bit different cut, when one object included in another: 1 and i get 2 But your method works perfectly.

KeithSloan commented 1 year ago

I do a bit different cut, when one object included in another: 1 and i get 2 But your method works perfectly.

Okay - Now able to reproduce the problem you are seeing, will see what we can do.

mhindi2 commented 1 year ago

Booleans should be between objects that have a SHAPE. App::Parts really should not be used as the base and tool of boolean operations, even if they some times seem to work. So the items in the last figure that should be selected for cutting are the GMLElTube and GDMLElTube001 and not the Parts they are in.

KeithSloan commented 1 year ago

Booleans should be between objects that have a SHAPE. App::Parts really should not be used as the base and tool of boolean operations, even if they some times seem to work. So the items in the last figure that should be selected for cutting are the GMLElTube and GDMLElTube001 and not the Parts they are in.

Note: That is not how it is currently coded. The selection is checking for a part.

If I switch to using selecting of GDMLobjects, what should happen about the Placement of the Part of the second Object? Ignore? or find parent?

mhindi2 commented 1 year ago

I would check for shape, not GDMLObject (hasattr(obj, 'Shape')). I believe the current code (that I fixed!) takes into account BOTH placements, but I will double check.

KeithSloan commented 1 year ago

But it could have a Shape and not be a GDML Object.

"The code you fixed" Is that in your repro?

mhindi2 commented 1 year ago

Yes, that's the hole point. The user should be able to do booleans with any solid, not just GDML objects. One should think of GDMLobjects as new specialized solids, but boolean operations should operate on any solid, not just the specialized solid.

The last branch I pushed and that you already merged into Main already has the code to to take into account the placement of both tool abd base Parts. Check the code in BooleanCutFeature.

KeithSloan commented 1 year ago

"Yes, that's the hole point. The user should be able to do booleans with any solid, not just GDML objects. One should think of GDMLobjects as new specialized solids, but boolean operations should operate on any solid, not just the specialized solid."

But when the Non GDML object with a Shape gets exported, how is that handled? surely better to have a GDMLBoolean that will only allow booleans of GDMLObjects.

Things can have Shapes, that have no GDML equivalent,

mhindi2 commented 1 year ago

Under the Part workbench if you try a cut with a Part that has sub parts:

Screenshot_20230404_090126

You get the error

09:02:12 Traceback (most recent call last): File "", line 1, in <class 'ValueError'>: 'Group' is not part of the enumeration in Unnamed1#Cut.ViewObject.DisplayMode

and the following is produced

Screenshot_20230404_085150

Now our current code only selects the first of the to objects and would work. Should discuss this offline before deciding on best approach.

KeithSloan commented 1 year ago

"Should discuss this offline before deciding on best approach."

AGREED

mhindi2 commented 1 year ago

@KeithSloan

We do export non GDML objects and booleans thereof and have been for a very long time. Extrusions, Revolutions, ... etc.

KeithSloan commented 1 year ago

"We do export non GDML objects and booleans thereof and have been for a very long time. Extrusions, Revolutions, ... etc."

So the export code will handle a boolean of a GDML object and Extrusion or Revolution?

But there is nothing to stop the user creating a non supported Shape will the export code just ignore, report the issue?

mhindi2 commented 1 year ago

Yes, the export code does handle booleans of GDML and non GDML objects, as long as there is an exporter for that particular solid. At present if there is no exporter, the solid will be skipped. We should add a message to that effect. Check your email.

mhindi2 commented 1 year ago

Yes, we currently do support export of booleans of GDML and (supported) non-GDML solids: Screenshot_20230404_093329

In case it is hard to see, above is a cut between a GDMLBox and a sphere (Part sphere, not GDMLSphere). Export works fine.

mhindi2 commented 1 year ago

I do a bit different cut, when one object included in another: 1 and i get 2 But your method works perfectly.

Doing a boolean operation between a volume and one of its descendants is really a little strange. If you do the operation under the Part workbench, you will get a result, but even there, the Part workbench complains:

15:54:13 Traceback (most recent call last): File "", line 1, in <class 'ValueError'>: 'Group' is not part of the enumeration in AlexTest#Cut.ViewObject.DisplayMode 15:54:13 Part::Cut: Link(s) to object(s) 'LV_EllipticalTube LV_EllipticalTube001' go out of the allowed scope 'Cut'. Instead, the linked object(s) reside within 'worldVOL worldVOL'.

Best have the operands of the boolean operation on the same level, not one contained within the other

Alex2671 commented 1 year ago

Actually, export to gdml far from ideal. Some details should be tuned after saving to gdml, but i fine with it. More crucial, there is objects that permanently lose their position, etc. For example GEARS.gdml.zip For now ELTube110 and Box098 has y position translation on -70 mm and -140 mm accordingly. Whereas initially Box099 had +70 mm translation through the y axis. After editing of placement, there is no changes occurs.

KeithSloan commented 1 year ago

Actually, export to gdml far from ideal. Some details should be tuned after saving to gdml, but i fine with it. More crucial, there is objects that permanently lose their position, etc. For example GEARS.gdml.zip For now ELTube110 and Box098 has y position translation on -70 mm and -140 mm accordingly. Whereas initially Box099 had +70 mm translation through the y axis. After editing of placement, there is no changes occurs.

Please could you supply the FreeCAD file for GEARS that you are exporting - Thanks

Alex2671 commented 1 year ago

Actually, export to gdml far from ideal. Some details should be tuned after saving to gdml, but i fine with it. More crucial, there is objects that permanently lose their position, etc. For example GEARS.gdml.zip For now ELTube110 and Box098 has y position translation on -70 mm and -140 mm accordingly. Whereas initially Box099 had +70 mm translation through the y axis. After editing of placement, there is no changes occurs.

Please could you supply the FreeCAD file for GEARS that you are exporting - Thanks

ger.FCStd.txt As you can verify, there are two types of fails: N250 is possible to be tuned, and N10, N614 as unchangeable objects.

mhindi2 commented 1 year ago

@Alex2671

ger1.FCStd.txt

Could you please check if the attached file works. There seems to be a problem (on our side) of exporting two different solids but with the same name. I have changed the name of the base solid in N1 from GDMLElTube_GDMLElTube to GDMLElTube_GDMLElTube000.

With my version of geant (geant4.10.07) I get display artifact when the cutting solid has a matching surface with the solid being cut, so I increased the z-dimension of the cutting solid to avoid that distracting artifact. That should have no effect on the simulation.

We will work on fixing the naming roblem.

I don't know what you mean "N10, N614 as unchangeable objects".

KeithSloan commented 1 year ago

I am trying to create simpler test cases.

If I extract N10 with Box098 I get GEARS-N10-Box098.FCStd.txt

And see that N10 has Placement (0,0,849) the common items Placement (0,0,0) Box99 ( 0,70,0) Box98 (0,0,0) ElTube(0,0,0)

Are you saying Eltube had pos -70 and Box98 had a Placement -140 before the Common operation.

KeithSloan commented 1 year ago

If I look at GEARS.gdml

I get

<solids>
    <box name="Box098" x="50.0" y="400.0" z="202.0" lunit="mm"/>
    <box name="Box099" x="300.0" y="100.0" z="202.0" lunit="mm"/>
    <eltube name="GDMLElTube110" dx="60.5" dy="60.5" dz="101.0" lunit="mm"/>
    <intersection name="intersection_intersection_intersection_intersection_intersection_Common">
      <first ref="Box099"/>
      <second ref="GDMLElTube110"/>
      <positionref ref="P-GDMLElTube1101"/>
      <rotationref ref="identity"/>
    </intersection>
    <intersection name="intersection_intersection_intersection_intersection_intersection_Common058">
      <first ref="intersection_intersection_intersection_intersection_intersection_Common"/>
      <second ref="Box098"/>
      <positionref ref="P-Box0982"/>
      <rotationref ref="identity"/>
    </intersection>
  </solids>

I am not seeing a name like with lots of repeats of intersection on export of GEARS-N10-Box098.FCstd


Do you have any clues as to why the names have intersection repeated a number of times?
Has there been a number of imports/exports of the file.
KeithSloan commented 1 year ago

If I cut the file down to N1 and N2, I am not seeing a duplicate Solid name

<solids>
    <box name="WorldBox" x="800.0" y="800.0" z="800.0" lunit="cm"/>
    <eltube name="GDMLElTube000" dx="52.5" dy="52.5" dz="15.0" lunit="mm"/>
    <eltube name="GDMLElTube001" dx="20.2" dy="20.2" dz="16.0" lunit="mm"/>
    <subtraction name="subtraction_subtraction_Cut">
      <first ref="GDMLElTube000"/>
      <second ref="GDMLElTube001"/>
      <positionref ref="center"/>
      <rotationref ref="identity"/>
    </subtraction>
    <eltube name="GDMLElTube002" dx="52.5" dy="52.5" dz="10.0" lunit="mm"/>
    <eltube name="GDMLElTube003" dx="34.45" dy="34.45" dz="11.0" lunit="mm"/>
    <subtraction name="subtraction_subtraction_Cut001">
      <first ref="GDMLElTube002"/>
      <second ref="GDMLElTube003"/>
      <positionref ref="center"/>
      <rotationref ref="identity"/>
    </subtraction>
  </solids>

GEARS-N1-N2.FCStd.txt GEARS-N1-N2-worldVOL.gdml.txt

in the FreeCAD file a duplicate of subtraction in the name has occurred would like to know what you are doing to make this happen, repeat of operation, export import?

mhindi2 commented 1 year ago

@KeithSloan

N250 and N1 were creating solids with the same name. I think that was the only instance of that. If you removed those, then the name clash would disappear. I wanted @Alex2671 to test the file I sent him so he can tell us of any remaining issues. Removing volumes is not a good test.

KeithSloan commented 1 year ago

@KeithSloan

N250 and N1 were creating solids with the same name. I think that was the only instance of that. If you removed those, then the name clash would disappear. I wanted @Alex2671 to test the file I sent him so he can tell us of any remaining issues. Removing volumes is not a good test.

Well I cut the main file down to N1 and N250 and do not see the name clash, surely that should not disappear.

I find it difficult to spot things with the complete file.

<solids>
    <box name="WorldBox" x="800.0" y="800.0" z="800.0" lunit="cm"/>
    <eltube name="GDMLElTube000" dx="52.5" dy="52.5" dz="15.0" lunit="mm"/>
    <eltube name="GDMLElTube001" dx="20.2" dy="20.2" dz="16.0" lunit="mm"/>
    <subtraction name="subtraction_subtraction_Cut">
      <first ref="GDMLElTube000"/>
      <second ref="GDMLElTube001"/>
      <positionref ref="center"/>
      <rotationref ref="identity"/>
    </subtraction>
    <eltube name="GDMLElTube002" dx="52.5" dy="52.5" dz="10.0" lunit="mm"/>
    <eltube name="GDMLElTube003" dx="34.45" dy="34.45" dz="11.0" lunit="mm"/>
    <subtraction name="subtraction_subtraction_Cut001">
      <first ref="GDMLElTube002"/>
      <second ref="GDMLElTube003"/>
      <positionref ref="center"/>
      <rotationref ref="identity"/>
    </subtraction>
    <eltube name="GDMLElTube" dx="8.5" dy="8.5" dz="0.25" lunit="mm"/>
  </solids>

GEARS-N1-N250.FCStd.txt GEARS-N1-N250-worldVOL.gdml.txt

mhindi2 commented 1 year ago

The file I sent already had the name change (I changed the name GDMLElTube to GDMLElTube000). I wanted a test on that file to see if there are remaining problems. You will see the clash if you used his original file.

On Mon, Apr 17, 2023, 10:58 AM Keith Sloan @.***> wrote:

@KeithSloan https://github.com/KeithSloan

N250 and N1 were creating solids with the same name. I think that was the only instance of that. If you removed those, then the name clash would disappear. I wanted @Alex2671 https://github.com/Alex2671 to test the file I sent him so he can tell us of any remaining issues. Removing volumes is not a good test.

Well I cut the main file down to N1 and N250 and do not see the name clash, surely that should not disappear.

I find it difficult to spot things with the complete file.

— Reply to this email directly, view it on GitHub https://github.com/KeithSloan/GDML/issues/108#issuecomment-1511836109, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWV3NCBPPEVFCJ3WEPWCD4LXBWACZANCNFSM6AAAAAAWNKIWNU . You are receiving this because you commented.Message ID: @.***>

KeithSloan commented 1 year ago

This could get tricky. FreeCAD only ensures Objects Names are unique and using those on Booleans is not good news.

Not sure how a name of GDMLElTube_GDMLElTube got created, looks like it was an import of a gdml file with a cut/subtraction.

Don't really want to use full label on export or drop GDML{Object} prefix from Label when creating.

Plus users can always rename Labels so could end up duplicate.

Will have to sleep on it.

Alex2671 commented 1 year ago

I am trying to create simpler test cases.

If I extract N10 with Box098 I get GEARS-N10-Box098.FCStd.txt

And see that N10 has Placement (0,0,849) the common items Placement (0,0,0) Box99 ( 0,70,0) Box98 (0,0,0) ElTube(0,0,0)

Are you saying Eltube had pos -70 and Box98 had a Placement -140 before the Common operation.

I mean if you will import to gdml and open this gdml with Freecad you will observe what i said before. I probably misled you because I continued the discussion in this topic. This question is not about common operations, but about converting to gdml.

Alex2671 commented 1 year ago

@Alex2671

ger1.FCStd.txt

Could you please check if the attached file works. There seems to be a problem (on our side) of exporting two different solids but with the same name. I have changed the name of the base solid in N1 from GDMLElTube_GDMLElTube to GDMLElTube_GDMLElTube000.

With my version of geant (geant4.10.07) I get display artifact when the cutting solid has a matching surface with the solid being cut, so I increased the z-dimension of the cutting solid to avoid that distracting artifact. That should have no effect on the simulation.

We will work on fixing the naming roblem.

I don't know what you mean "N10, N614 as unchangeable objects".

If under "working" you mean test in geant? I have not tested all construction yet, but isn't intersections via exporting to gdml would affect on simulation work? Your example looks in FCStd format like image but in gdml: image

mhindi2 commented 1 year ago

Screenshot_20230417_134721

ger1-worldVOL.gdml.txt

I don't see what you see. Although there still seem to be differences it is hard for me to tell what they are without a lot of work.

If under "working" you mean test in geant?

Yes

but isn't intersections via exporting to gdml would affect on simulation work? Your example looks in FCStd format like

What I was referring to is that if you cut, say a box with dimensions x=50,y=50, z=50, with a gdml tube (apropriately shifted in the x, y plane, with dimensions, of, say dx=30, dy=30, dz=50/2, geant will sometimes display the common (infinitely thin) surfaces at +/- z. For example, here is a screen shot of your original file and its exported gdml opened in geant:

Screenshot_20230417_140217

Note all the extra disks that geant displays, that FreeCAD does not. These are the common surfaces at the end that the geant display does not "cut away". These are in principle with zero thickness so should not affect the simulation, but they do look ugly. If you make the cutting tool slighly bigger, they disappear. That also does not affect the simulation (for most situations), because the way testing for a cut like, say, solid1 subtract solid2 is: is track point in solid1 but not in solid2. You can make solid2 larger, without affecting the result of the test. This is best illustrated with some images:

Consider a tube cut by a box, where the height of the box exactly matches the height of the tube (this, by the way, is how almost all the cuts in your original file are):

cut1

If you export as a gdml and open in geant (at least on my system), this is what you get:

Screenshot_20230417_141604

Notice the extra surface at the bottom in the geant display. This surface is not present in the FreeCAD display.

Now if you increase the z-dimension of the cutting box:

cut3

The resulting cut solid is exactly the same as before, but now geant displays is properly.

Screenshot_20230417_141933

The file (ger1.FCStd) that I posted before has the z-dmension of all the cutting box slighly increased to avoid the ugly geant displays. The resulting solids are exactly what was intended and should not have any effect on the simulation. So, in short, try to avoid cutting with object that share common surfaces.

Alex2671 commented 1 year ago

Lets watch on simplest example: test.FCStd.txt We see sc1

after export -> import, i hope we all observe this:

sc2 N36 and N37 are falling down. Whereas N35 is not, while it is almost the same as N36 and N37. So why is the difference happens? Because i do the following steps designing N[*] element:

1) intersection between boxes; 2) On this stage i can chose between tube-intersection intersection(as in N35 case) and intersection-tube intersection(as in N36, N37 cases).

Conclusion: strangeness occurs when i chose wrong path at 2). In my opinion, this ways are identical and must give the same result.

mhindi2 commented 1 year ago

OK, I finally am able to reproduce the problem you are observing, and I think I know its cause. I will try to fix it in the next few days, but Keith has to merge any fixes into the main github and he is busy until May 15th. I will try to post a temporary fix into my github later on.

For the future, it would help in isolating the problem if you try to identify whether the problem occurs on export or on import. That is, rather than compare two images after exporting, then re-importing the exporterd file, as you do above, compare images of the original FreeCAD file with what geant4 displays. If there is a difference, then we know the problem is in the exporter FreeCAD->gdml. If there is no difference, but after you import the gdml file you see a difference then the problem is in the importer gdml->FreeCAD. In the problem you report, the problem is that the exported gdml file is wrong. Unfortunately, with your original file, there was an additional problem, not related to the above, that made it even more difficult for me to undestand what problem you were observing.

Thanks for finding and reporting the problem!

mhindi2 commented 1 year ago

@Alex2671

The bug in the export occurs when (1) The first element in the boolean (the base) has a non-zero displacement or rotation AND (2) the boolean is subsequently used in another boolean. So, to avoid the bug, and until I find a way to fix it, please construct booleans by using a base with no displacement. It is always possible to achieve the desired result by a suitable displacement of the second element (the tool) and placing the result of the boolean appropriately.

The attached file does this and the result exports correctly, as the screenshot shows. test.FCStd.txt first_0-displacement

The difficulty arises because in GDML booleans are constructed by specifying the displacement (and/or rotation) of the second element relative to the first. The first element cannot have its own displacement, but the overall boolean could be displaced. In FreeCAD the elements of the boolean can have their own displacements, specified relative to the origin, not one relative to the other.

KeithSloan commented 1 year ago

@Alex2671 - Have updated importGDML so that if re-importing it should create the FreeCAD document as exported.

Does not fix things for your current file but should prevent boolean names and GDMLObject names becoming more complex and avoid some duplicate names looking forward.