moose
moose copied to clipboard
General multiapp field transfer
Created a new transfer
Working on these features
-
Block restricted transfers #16427.
-
MultiApp transfers support array and vector variables #16429
Job Documentation on 3d613e2 wanted to post the following:
View the site here
This comment will be updated on new commits.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Nope
Check where we are
Job Coverage on 3d613e2 wanted to post the following:
Framework coverage
ef49eb | #17417 3d613e | ||||
---|---|---|---|---|---|
Total | Total | +/- | New | ||
Rate | 84.62% | 84.70% | +0.08% | 89.40% | |
Hits | 81980 | 82830 | +850 | 961 | |
Misses | 14896 | 14963 | +67 | 114 |
Modules coverage
Coverage did not change
Full coverage reports
Reports
-
framework
-
chemical_reactions
-
combined
-
contact
-
electromagnetics
-
external_petsc_solver
-
fluid_properties
-
fsi
-
functional_expansion_tools
-
geochemistry
-
heat_conduction
-
level_set
-
misc
-
navier_stokes
-
optimization
-
peridynamics
-
phase_field
-
porous_flow
-
ray_tracing
-
rdg
-
reactor
-
richards
-
scalar_transport
-
solid_properties
-
stochastic_tools
-
tensor_mechanics
-
thermal_hydraulics
-
xfem
Warnings
-
framework
new line coverage rate 89.40% is less than the suggested 90.0%
This comment will be updated on new commits.
Job Precheck on a59f268 wanted to post the following:
Your code requires style changes.
A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:
curl -s https://mooseframework.inl.gov/docs/PRs/17417/style.patch | git apply -v
Alternatively, with your repository up to date and in the top level of your repository:
git clang-format 4ab280bda315b1c517a87a11c3f0742caab75668
Is it possible to replace the existing mesh function and nearest node transfers with the new versions here?
Is it possible to replace the existing mesh function and nearest node transfers with the new versions here?
That is the plan. I can not see why not. But need to make sure the replacement happen gradually. It is not a good idea to remove old ones while new ones have not been widely utilized yet
@GiudGiud @lindsayad should help me get the PR in, lol
Is it possible to replace the existing mesh function and nearest node transfers with the new versions here?
That is the plan. I can not see why not. But need to make sure the replacement happen gradually. It is not a good idea to remove old ones while new ones have not been widely utilized yet
I agree with that. I m going to start working on getting this in next week. This looks awesome already, just needs some finishing touches.
Job Precheck on 3c009f6 wanted to post the following:
Warning: This PR changes repo size by 8.66 MiB.
All jobs on 20d5388 : invalidated by @GiudGiud
if civet isnt upset about working during curtailment this will be green
Coverage wise the only shocking thing is the lack of coverage on the overlap/value conflict/floating point indetermination search in parallel. This is actually likely just because the recipe doesn't cover all the parallel recipes, as it's explicitly targeted by a test
Some profiles with tests at higher refinement.
Serial UO_3d.pdf NN_3d.pdf UO_2d.pdf UO_BR_2d.pdf (most interesting) UO - 3D test with r5 : 16,538,562 dofs UO - 2D test with r6 : 1,640,962 dofs UO - 2D test with r7 + block restriction + higher order variables : 5M dofs NN - 3D test with r4 : 2,086,882 dofs
For UO transfer we see the point locator creation and that is it. We could avoid creating them when they are not needed (no block or boundary restriction OR NN transfer)? Doesn't seem like a huge gain.
The slow parts are initial conditions, the BoundaryNodeIntegrityCheckThread and the mesh refinement in 3D
For block restriction : not a big cost overall
For higher order variable: quite a cost from the generic projector, seems to be a bunch of mallocs and appends. This should be done on every iteration though, there's no way around it. Maybe if there's a way to update the projector at a lower cost, not sure. I would need to build libmesh from source to investigate at this point.
The append
that show up in the profile could be vector push_back
getting expensive if the vectors are not sized correctly, but looking at the routine there's only one.
Maybe with more timesteps we will start seeing the transfers in profiling I think we can wait until we get feedback on this from users
Parallel WIP I ll update when I get back, maybe
There's definitely a lot of short vector use in GenericProjector
that I'd think could get a lot faster from a Small-Size Optimization pass ... but IIRC when we tried that for the vectors in AD we didn't see any speedup at all, so my intuition isn't to be trusted here.
Sounds good. The failure with Griffin is trivial btw, this is ready for review
Score! Those Distributed mesh sweep issues look unrelated too ... though I'm a bit concerned about what might have caused them to regress.
I'll start going through the code tomorrow.
The strings-for-indexing comments from @friedmud are still applicable issues?
I think the various profiles I posted show these are negligible
@roystgnr @loganharbour the distributed mesh failures here are not tied to this PR. They are tied to me turning on possibly an old distributed mesh sweep recipe. The current one on next does 2,4,8,16. https://civet.inl.gov/job/1286241/ The one that is failing here is 2,4,6,8,10,12,14,16. https://civet.inl.gov/job/1279751/ 10 and 12 are failing on tests unrelated to my PR.
I ll try to fix them but maybe not on this PR
Get ready, last time I got 900 emails about the size of the repo
It seems with those absurd build errors I missed that the nearest_app tests are failing at some ranks. I ll try to find time to look at them.
We cant avoid perturbed lookups. This comes from the multiplicity of target sub app positions.
I think a parameter to the map would need to be part of the template for it since the maps are not class members of General Field Transfer. I ll see what I can do. I might just forbid this transfer for small meshes or force an error for near 0 insertions.
Forbidding the transfer for (very) small meshes might be reasonable. Forcing an error for near-0 insertions, not so reasonable; it's too easy to do some mesh manipulation that you expect to leave a vertex at 0 but then run into FP error and end up with a vertex at 3e-12
or so instead.
since we are only doing a (=0) + b for insertion then (a+b) - b for retrieval I think we are guaranteed that at exactly 0 we will never have floating point errors? Is b - b = 0 guaranteed by floating point math?
now if you built your mesh at 1e-13 because your mesher output 1e-13 instead of 0 for the first point, then we're out of luck.
but we can just error then. And then whoever wants that will have to offset their mesh by (1, 1, 1) to make it work.
I think I have a lead on setting a dimension on the map. I think it s possible now, thanks to the struct around the map
now if you built your mesh at 1e-13 because your mesher output 1e-13 instead of 0 for the first point, then we're out of luck.
This literally is one use case I'm thinking of. I discovered while bug hunting at one point that Cubit was introducing epsilon FP error even to what ought to have been a very simple cartesian grid.
But there are any number of other cases that will end up there. Not long ago someone tried to do a 180 degree rotation and expected a precise result and got thwarted by the fact that sin(double(pi))
is not zero.
@roystgnr I worked around the issue. See the commit message for the explanation on the design
Job Precheck on 3d613e2 wanted to post the following:
Warning: This PR changes repo size by 14.43 MiB.
Yeah, that's an approval. I think we're good to merge once whatever's going on with Griffin gets sorted.
wouhou!
Thanks for the careful reviews @roystgnr and thanks for the excellent work @fdkong !! It was a pleasure building on it
Thanks @GiudGiud . You have all credits!
Great work.
Thanks to all of you ;-)