Added cells supporting unions #134
Conversation
Required a change to dictionary reading to allow mixed character/number reading. Also fixed some comments in nearby files.
ChasingNeutrons
requested review from
Mikolaj-A-Kowalski and
valeriaRaffuzzi
| character(*), parameter :: EASYCELL_DEF = "& | ||
| & id 2; type unionCell; surfaces [-13 -15 ]; filltype outside; " | ||
| character(*), parameter :: COMPLEXCELL_DEF = "& | ||
| & id 3; type simpleCell; surfaces [< -13 -15 > # < -4 : -5 : -6 > ]; filltype outside; " |
There was a problem hiding this comment.
shouldn't this be unionCell too?
There was a problem hiding this comment.
The cell % init procedure doesn't read type, so that could be removed actually.
There was a problem hiding this comment.
Very good spot. I will remove it from the equivalent simpleCell test for now too.
| call charToDict(dict, SURF_DEF) | ||
| call surfs % init(dict) | ||
| call dict % kill() | ||
| call charToDict(dict, EASYCELL_DEF) | ||
| call easyCell % init(dict, surfs) | ||
| call dict % kill() | ||
| call charToDict(dict, COMPLEXCELL_DEF) | ||
| call complexCell % init(dict, surfs) |
There was a problem hiding this comment.
| call charToDict(dict, SURF_DEF) | |
| call surfs % init(dict) | |
| call dict % kill() | |
| call charToDict(dict, EASYCELL_DEF) | |
| call easyCell % init(dict, surfs) | |
| call dict % kill() | |
| call charToDict(dict, COMPLEXCELL_DEF) | |
| call complexCell % init(dict, surfs) | |
| call charToDict(dict, SURF_DEF) | |
| call surfs % init(dict) | |
| call dict % kill() | |
| call charToDict(dict, EASYCELL_DEF) | |
| call easyCell % init(dict, surfs) | |
| call dict % kill() | |
| call charToDict(dict, COMPLEXCELL_DEF) | |
| call complexCell % init(dict, surfs) |
What about adding spaces to make it more legible?
Geometry/Cells/unionCell_class.f90
Outdated
|
|
||
|
|
||
| !! | ||
| !! CSG cell defined by intersections and unions of halfspaces |
|
To answer the questions in the description:
|
|
Thanks for the comments. I think, at least for this PR, I will keep simple cells and open an issue to remove them. In part this is because getting rid of it means changing all of the inputs which use them! |
That's ok, we can make an issue. Merge? |
There is a new cell type which supports unions, complements, and parentheses. It uses infix notation, inspired by the results of Paul Romano et al.'s recent paper.
There are several style/code structure questions that should be resolved here.
First, this cell essentially supercedes the simpleCell. Do we want to get rid of the simpleCell? It might be code duplication otherwise and I can't think of a very good case for keeping it. Maybe we wait a little while until we're comfortable with this one.
Second, for the operators necessary, reading them in alongside the surfaces required a deviation in input parsing/reading. Currently lists do not allow mixed number/character arrays. Hence I created the 'tokenArray' to contain arbitrary white-space separated characters of any format. These are read into a character array - I convert the numbers back to integers inside unionCell. It is denoted by using square brackets. Is this acceptable? Is there a nicer way of doing this? This also requires that surfaces and operators be delimited by spaces - maybe this isn't strictly necessary if I put some more logic inside the unionCell itself, for example. I would especially like to resolve this quickly because it will affect the input format and it would be great to settle that quickly so that we can start using this, e.g., with csg2csg.
I have also written a unit test and changed some comments in other nearby files.