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 CSG solids #1137

Merged
merged 6 commits into from
Mar 8, 2024
Merged

Add CSG solids #1137

merged 6 commits into from
Mar 8, 2024

Conversation

sethrj
Copy link
Member

@sethrj sethrj commented Mar 1, 2024

CHAINED on #1142

This adds basic solids, which are shapes with interiors optionally subtracted, and azimuthal sections optionally removed.

@sethrj sethrj added enhancement New feature or request orange Work on ORANGE geometry engine labels Mar 1, 2024
@sethrj sethrj marked this pull request as ready for review March 7, 2024 12:59
@sethrj
Copy link
Member Author

sethrj commented Mar 7, 2024

@elliottbiondo This is the end of the queue!! 🎉

Copy link
Contributor

@elliottbiondo elliottbiondo left a comment

Choose a reason for hiding this comment

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

Since this is the last PR in the queue, could you add a brief design document? Maybe you did in a previous MR after I already approved and didn't see it.

src/orange/orangeinp/Solid.hh Outdated Show resolved Hide resolved
src/orange/orangeinp/Solid.hh Show resolved Hide resolved
src/orange/orangeinp/Solid.cc Show resolved Hide resolved
src/orange/orangeinp/Solid.hh Show resolved Hide resolved

//---------------------------------------------------------------------------//
/*!
* A shape that is hollow, is truncated azimuthally, or both.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say "interior excluded region" and/or a wedge subtracted.

I also don't understand why it couldn't be neither, though I see that that is how it is coded.

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'm deliberately trying to prevent two Object types from being the same representation unless they compare equal... maybe I'll relax that eventually, but I think for now I don't think it's a problem to keep the restriction. Or do you think it's better to have a "Solid" that's the same as a "Shape", and later if we need to we can add a "simplify" method that'll let solids decay to shapes? That would remove some logic from the Geant4 translator...

Copy link
Contributor

Choose a reason for hiding this comment

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

So if a geometry consists of only simple cuboids for example (with no internal regions or wedges) then solids are never created? In a sense it would be nice to have geometry elements go through the same levels of abstraction, but it might not be worth it if that adds a bunch more code that is essentially a no-op. To weight in further I think need so study the forthcoming design diagram/document, because I am not clear on what exactly happens on each level of abstraction.

Copy link
Member Author

Choose a reason for hiding this comment

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

just Shapes, not Solid. The abstraction is that they're both Object interfaces. So in Geant4 a "cone solid" could produce either a ConeShape in its simplest form or a ConeSolid if it's truncated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay I see. The nomenclature here is difficult especially since Geant4 nomenclature clashes with ORANGE nomenclature (recognizing that may be unavoidable).

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I've tried to delineate it a bit: "shape" is going to be for the simpler objects, "solids" are the weird Geant4 ones. See #1078 (comment)

src/orange/orangeinp/Solid.hh Outdated Show resolved Hide resolved
@sethrj
Copy link
Member Author

sethrj commented Mar 8, 2024

@elliottbiondo take a look at doc/main/api/orange.rst... that part describes the user-facing parts and some of the construction logic. I've got most of the basic "proto" construction written which is the final piece of the puzzle... perhaps we could do a detailed review then?

@sethrj sethrj merged commit 7eae521 into celeritas-project:develop Mar 8, 2024
21 checks passed
@sethrj sethrj deleted the csg-solid branch March 8, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request orange Work on ORANGE geometry engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants