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

First stab at importing tracing routines #3432

Open
wants to merge 30 commits into
base: devel
Choose a base branch
from

Conversation

loganharbour
Copy link
Member

No description provided.

@moosebuild
Copy link

moosebuild commented Nov 8, 2022

Job Coverage on ae116d8 wanted to post the following:

Coverage

09d0e4 #3432 ae116d
Total Total +/- New
Rate 59.74% 59.89% +0.15% 92.39%
Hits 48341 48662 +321 328
Misses 32578 32594 +16 27

Diff coverage report

Full coverage report

This comment will be updated on new commits.

@loganharbour
Copy link
Member Author

@roystgnr everything here is complete and fully tested - this is definitely only a chunk of it, but could be reviewed now.

@loganharbour loganharbour changed the title Import ray tracing routines First stab at importing tracing routines Nov 10, 2022
@loganharbour
Copy link
Member Author

I'll also add that I think I have the most of the non-tool changes made, that is the stuff to Elem and the derived classes. That was my goal here - get those in first, and then add a few tools.

include/geom/cell_inf_prism.h Outdated Show resolved Hide resolved
include/geom/elem.h Show resolved Hide resolved
include/geom/elem_extrema.h Outdated Show resolved Hide resolved
include/geom/elem_extrema.h Outdated Show resolved Hide resolved
Makefile.in Show resolved Hide resolved
include/geom/elem.h Outdated Show resolved Hide resolved
include/geom/elem.h Outdated Show resolved Hide resolved
include/geom/elem.h Show resolved Hide resolved
include/geom/intersection_tools.h Outdated Show resolved Hide resolved
src/geom/intersection_tools.C Outdated Show resolved Hide resolved
@loganharbour
Copy link
Member Author

@roystgnr - good to go.

@loganharbour
Copy link
Member Author

Re the tolerance stuff: the last few tests run these scaled with element features on the order of [1e-8,1e8]

const Real tol)
{
for (const auto v : elem.vertex_index_range())
if (elem.point(v).relative_fuzzy_equals(p, tol))
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a proper relative tolerance. Here relative_fuzzy_equals will use p+elem.point(v) to compare against, which is likely to be way too big in some cases (a tiny element far from the origin) and is going to be effectively zero in other cases (a huge element with a vertex at the origin).

I'd normally use elem.hmax() as a scale here, but that might be too expensive for you? If so, it wouldn't be unreasonable to have something like Elem::cheap_scale() that returns something like an l1 norm (how do we not already have that for TypeVector...) of (elem.point(1)-elem.point(0)). Kind of arbitrary for anisotropic meshes but so is the choice of hmax() over hmin().

for (const auto i : make_range(elem.n_vertices_on_side(s)))
{
const auto v = side_node_map[i];
if (elem.point(v).relative_fuzzy_equals(p, tol))
Copy link
Member

Choose a reason for hiding this comment

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

Similar tolerance issue here.

@gridley
Copy link
Contributor

gridley commented Jan 30, 2023

This is exciting! We'll be able to add volumetric mesh geometries to OpenMC after this is merged (at the moment only surface mesh geometries are allowed). OpenMC already optionally links to libmesh for volume mesh tallies, but this will make multiphysics coupling substantially more straightforward. 😄

@loganharbour
Copy link
Member Author

Oh hi @gridley. Yeah, this is completely OpenMC driven after some conversations with @pshriwise. I've got another branch pulling over the rest of the routines, just haven't made a PR yet. On that note - thanks for the reminder to finish this up 😆

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.

4 participants