[BUG] - Surface number 0 is used in Cell definition

[shimwell]

Describe the bug

Looks like the surface numbers used in the cell definition don't match the surface numbers.

Expected behavior

Surface ids start at 1 but the cells use surface with id 0

Error message

No error in geound but will not work in transport

Screenshots

Easy to see in the openmc python code as VS code highlights the undefined variables S0

Also in the mcnp and other codes

ssets/8583900/73a3c1cf-6175-4fb5-a6fa-61da5d51ce50)

Please complete the following information):

• OS: linux
• GEOUNEd dev
• Python version 3.11

Additional context

I should add some tests to check cells don't use no existent surface numbers

[shimwell]

min_example.zip
attached is a step file an python script that reproduces the error on dev and main branch

Error also reported by @py1sl

[shimwell]

screen shot of the geometry

[akolsek]

The step model appears to contain splines. Please double check.

[KBGrammer]

This same error pops up for me during simplification and I've seen it in the resulting MCNP file. I think it might also cause issues during void generation.

File "~/GEOUNED-1.1.0/src/geouned/GEOUNED/utils/boolean_solids.py", line 253, in build_c_table_from_solids
    if type(surfaces[surfaceList[0]]) is GeounedSurface:
KeyError: 0

My model does have spline surfaces. Are splines becoming a surface number of 0 in cell definitions?

Is there a way to make geouned soft-error on things? A "this is a spline, ignoring it, moving on" warning rather than a crash would be good.

I patched my local version to try to put a bandaid on it in build_c_table_from_solids, but 0 index keeps popping up in new spots. It probably needs cleaned up earlier than I'm trying to clean it up.

[shimwell]

I was planning to add a check that looked for spline models in the cells that are being converted and raised a ValueError if the cells contain splines.

I also think that if #204 is solved this would give us a way of filtering out cells with splines as it would allow arbitrary volume IDs to be skipped instead of a range of adjacent volume IDs

In other news I've also done a bit of digging into methods of automatically replacing splines which I ca report on here when it is more ready

[KBGrammer]

#204 would help as long as debug is on, right? Or would log files have enough information to show you which cell ID it failed on? It would let you easily see which cell failed and skip it on the next try. However, that becomes problematic if if fails on cell 100 of 1000 and then fails on 150 of 999 and then fails on 250 of 998, etc. You could do simple parallel and split it manually with the input and run 10 sets to find 10 failures, remove those, repeat, though.

It would be great if you could just tell it to work on a conversion and just flag/warn you about failed cells. If I've got a big model, it makes it easy to run overnight and come back tomorrow to see which bodies failed to convert, decide if I care, and fix it if I need to.

The 0 surface comes up a lot. I've been modifying the code locally to put in bandaids to try to get around them and then it pops up and causes problems elsewhere.

[shimwell]

If the log files don't show which cell the conversion failed on then that is another thing that should be fixed.

In generally think errors should stop the code when they are found and failing silently or just printing warnings can easily be overlooked and give the incorrect impression everything worked.

So I'm generally in favour of errors in conversion resulting in the code failing and stopping on the error with a nice message. The error message can contain the ID numbers of the solids with splines in them and it doesn't need to attempt a conversion to find the splines.

PR #204 would then allow you to then skip these spline containing solids by their ID number

[shimwell]

@KBGrammer here is some code that will identify all the solid ID numbers in a step file that contain spline edges.

If PR #204 gets merged then I can follow up that PR by adding the spline checking and reporting method into the cad loading functions.

import freecad
import FreeCAD
import Part
from FreeCAD import Import

filename = 'desplined.stp'  # chage to your own filename

cad_simplificado_doc = FreeCAD.newDocument("CAD_simplificado")
p0 = FreeCAD.ParamGet("User parameter:BaseApp/Preferences/Mod/Import")
p0.SetBool("UseLinkGroup", False)
p0.SetBool("ReduceObjects", False)
p0.SetBool("ExpandCompound", False)

p1 = FreeCAD.ParamGet("User parameter:BaseApp/Preferences/Mod/Import/hSTEP")
p1.SetBool("UseLinkGroup", False)
p1.SetBool("ReadShapeCompoundMode", True)

s = Part.Shape()
s.read(filename)
Solids = s.Solids

# This loop can be added to load.step.py to check for splines
# TODO account for skip solids
solid_ids_with_splines = []
for counter, individual_soild in enumerate(Solids):
    for edge in individual_soild.Edges:
        if edge.Curve.__str__() == '<BSplineCurve object>':
            solid_ids_with_splines.append(counter)
if len(solid_ids_with_splines) > 0:
    msg=f'The step file {filename} contains the spines in solid ID numbers {set(solid_ids_with_splines)}. Splines can not be converted directly to CSG geometry. You can either remove these solids from the step file or use the skip_solids argument to skip these solids ID numbers'
    raise ValueError(msg)

[KBGrammer]

That snippet works great for my purpose. Thank you very much for the suggestion!

[alexvalentine94]

solving in #210