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

Add support for transformed surface nums #97

Open
MicahGale opened this issue Aug 5, 2022 · 9 comments
Open

Add support for transformed surface nums #97

MicahGale opened this issue Aug 5, 2022 · 9 comments
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers

Comments

@MicahGale
Copy link
Collaborator

In GitLab by @tjlaboss on Aug 5, 2022, 15:55

When a TRCL transformation is applied to a cell bounded by a surface, a new surface number is generated like this:

TTT*1000 + SSS
  where SSS is the original surface number
    and TTT is the transformation   number

The new surface number may then be referenced directly by other cells. Example:

340 0 (94 -209 -340) fill=14 trcl=(0 0 0  0.7071 -0.7071 0  0.7071 0.7071 0  0 0 1) $ 
345 6666 -0.726 (340209:340340:-340094) (96 -204 -12900) $ Water gap between specimen universe and flow tube

This results in mcnpy.errors.BrokenObjectLinkError: Surface 340209 is missing from the input from the definition of: Cell 345.

@MicahGale
Copy link
Collaborator Author

Relevant to #24.

@MicahGale
Copy link
Collaborator Author

So how should we handle this case. My first thought is to make a copy of the surface, but then we immediately have any issue with race conditions for the numbers. Should we almost make a shadow object, that doesn't exist until you query about it?

@MicahGale
Copy link
Collaborator Author

I envy MCNP a little bit, their problem representation is effectively immutable, and they don't have to think about Mutation like we do.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Aug 5, 2022, 17:04

This is a weird feature to support. My initial thought was just use the fake-id as the key in the Surfaces collection:

>>> surfs = mcnpy.surface_collection.Surfaces(...)
>>> print(surf312.number)
312
>>> surfs[42312] = surf312  # for transform 42

...but I am pretty sure that will just get destroyed when updating pointers.

Copying it becomes a problem as soon as the original object mutates.

So having a shadow object seems to be the only solution.

@MicahGale
Copy link
Collaborator Author

Yeah I'm thinking it will point to the cell and surface. Most surface stuff will be a pass through to the lower object. The transform will be the cell's transform, and the number will be calculated on the fly. Should that avoid all issues? The annoying thing is that this will have to subclass Surface, which will make some things janky. This is where interface/trait based OOP and not inheritance would be nice.

What should the name be: IlluminatiSurface?

@MicahGale
Copy link
Collaborator Author

So other thought. #24 is basically trying to enforce their janky documentation. Which as far as I can tell. This case is the only one where you need to modify the number rules. So what if for #24 we just enforce the basic rules (like you can't go into the billions), and then use this to enforce number collision avoidance due to transforms, which is the route of the special restrictions.

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Aug 5, 2022, 17:42

Something like SurfaceView or TransformedSurface

@MicahGale
Copy link
Collaborator Author

In GitLab by @tjlaboss on Aug 5, 2022, 17:57

That makes sense. The TRCLn number limits only need to be enforced if they result in number conflicts.

@MicahGale
Copy link
Collaborator Author

Oh wait I just opened #68 for this, whoops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request An issue that improves the user interface. good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant