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

Fix close surfaces bug #2618

Closed
wants to merge 1 commit into from
Closed

Conversation

egor1abs
Copy link
Contributor

The problem manifests itself when two surfaces overlap, such as two spheres at the origin with the same radius. If you put the source in the center of the spheres, OpenMC will take a very long time to calculate the intersection of these surfaces by a particle, which significantly reduces performance. In the same conditions between MCNP and OpenMC there will be about 200 times difference in simulation time.
If we use a debugger, it becomes clear that the function that determines the distance traveled by the particle determines the value of the distance traveled between the spheres as ~1e-16, which is less than the computational error.
I am not sure that my solution to the problem in this case is universal, but I was able to achieve a significant performance improvement for the tasks I need. The point is that in the src/particle.cpp file in line 206 we can set the distance parameter so that a result less than 1e-12 is not considered further. Since 1e-12 meters is less than the size of an atom, it is unlikely that a distance less than an atom will be encountered.
I'm sure we can come up with a more universal solution to this problem, for example, by removing repeating surfaces beforehand, but at the moment I can only see such a solution to the problem.

Before:
=======================> TIMING STATISTICS <=======================
Total time for initialization = 1.4792e-02 seconds
Reading cross sections = 1.1796e-04 seconds
Total time in simulation = 1.0215e+02 seconds
Time in transport only = 1.0215e+02 seconds
Time in active batches = 1.0215e+02 seconds
Time accumulating tallies = 2.4692e-05 seconds
Time writing statepoints = 1.8115e-03 seconds
Total time for finalization = 3.5190e-06 seconds
Total time elapsed = 1.0216e+02 seconds
Calculation Rate (active) = 97896.6 particles/second
============================> RESULTS <============================

After:
=======================> TIMING STATISTICS <=======================
Total time for initialization = 1.3530e-02 seconds
Reading cross sections = 1.2147e-04 seconds
Total time in simulation = 8.5538e+01 seconds
Time in transport only = 8.5536e+01 seconds
Time in active batches = 8.5538e+01 seconds
Time accumulating tallies = 1.7564e-05 seconds
Time writing statepoints = 1.8315e-03 seconds
Total time for finalization = 2.6580e-06 seconds
Total time elapsed = 8.5552e+01 seconds
Calculation Rate (active) = 116907 particles/second
============================> RESULTS <============================

Added distance lower boundary 1e-12
@eepeterson
Copy link
Contributor

Hi @egor1abs thanks for looking into this and proposing a fix! Does the merge_surfaces property of the Geometry class serve the purpose you're looking for? It removes redundant surfaces from the geometry before exporting to xml.

@egor1abs
Copy link
Contributor Author

Hi @egor1abs thanks for looking into this and proposing a fix! Does the merge_surfaces property of the Geometry class serve the purpose you're looking for? It removes redundant surfaces from the geometry before exporting to xml.

Hi, thanks for your answer!
I'm using xml files transferred by the converter from MCNP to OpenMC made by @paulromano. Maybe that is indeed the problem, will definitely check it out!

@paulromano
Copy link
Contributor

Thanks for reporting this @egor1abs! In this case, I can add a new option in the MCNP converter to merge surfaces (right now it does not do it by default).

@paulromano
Copy link
Contributor

@egor1abs I just updated the openmc_mcnp_adapter to now remove redundant surfaces by default. Give it a shot and if you still have issues, let me know!

@paulromano paulromano closed this Jul 24, 2023
@egor1abs
Copy link
Contributor Author

@egor1abs I just updated the openmc_mcnp_adapter to now remove redundant surfaces by default. Give it a shot and if you still have issues, let me know!

Thanks @paulromano! I will check it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants