Some fixes for explicit3D ray tracing #357
Conversation
…etry class In the destructor function of Geometry class, materials should not be released in case they will be needed somewhere else. The materials could be freed explicitly by dict.clear() in python and delete in cpp.
…all newly created cells
src/Cell.cpp
Outdated
| @@ -299,7 +299,7 @@ double* Cell::getRotationMatrix() { | |||
| * @param units the angular units in "radians" or "degrees" (default) | |||
| */ | |||
| void Cell::retrieveRotation(double* rotations, int num_axes, | |||
| std::string units) { | |||
| std::string units) { | |||
There was a problem hiding this comment.
I know that the style guide says to leave 5 blank spaces, but it looks a lot better yo have the arguments on the second line line up with the parenthesis. We could change the guide lines.
void Cell::retrieveRotation(double* rotations, int num_axes,
std::string units) {
| @@ -1434,6 +1438,7 @@ void Cell::ringify(std::vector<Cell*>& subcells, double max_radius) { | |||
| Cell* ring = clone(); | |||
| ring->setNumSectors(0); | |||
| ring->setNumRings(0); | |||
| ring->removeSurface(zcylinder1); | |||
There was a problem hiding this comment.
Does this suffice to close issue #226 ?
There was a problem hiding this comment.
Not solve this issue completely. It is hard to remove the abundant outer box surfaces for outer most cells ( moderator usually). However, this could be improved more.
There was a problem hiding this comment.
I found when the number of sectors is 2, the code crashes.
There was a problem hiding this comment.
It is the problem when a point is on the surface. In this case, the evaluation value would be so tiny that it could be either minus or positive. Will be fixed in other PR.
There was a problem hiding this comment.
This removeSurfaces doesn't actually work. It's calling getSurfaces which returns a const map, so you can't delete from it.
No need to make a PR to fix it, all of this is gone with Regions
| std::map<int, Material*>::iterator iter; | ||
| for (iter = materials.begin(); iter != materials.end(); ++iter) | ||
| delete iter->second; | ||
|
|
There was a problem hiding this comment.
Add a if (_loaded_from_file) { instead, we can't go back to memory leaking the materials
src/Geometry.cpp
Outdated
| surf = (*s_iter).second->_surface; | ||
| all_surfs[surf->getId()] = surf; | ||
| surf = (*s_iter).second->_surface; | ||
| all_surfs[surf->getId()] = surf; |
src/LocalCoords.cpp
Outdated
| iter = iter->getNext(); | ||
| n++; | ||
| if (n > 1000) | ||
| log_printf(ERROR, "Infinite loop of coords"); | ||
| } | ||
| log_printf(DEBUG, "The LocalCoords is: %s\n", toString().c_str()); | ||
| log_printf(NORMAL, "The depth of the chain is %d \n", n); |
There was a problem hiding this comment.
Make it a debug log if necessary
|
@wuwenbin2006 I will further review this only when the fix is complete and when explicit ray tracing provides the same results as OTF ray tracing. |
The code crashed when I ran it in explicit3D ray tracing mode. Some bugs are fixed and the code ran smoothly but got the wrong results.