search

ORNL-CEES / DataTransferKit

A library for multiphysics solution transfer. ARCHIVED
https://datatransferkit.readthedocs.io/en/dtk-3.0/
BSD 3-Clause "New" or "Revised" License
47 stars 26 forks source link

Update arborx #570

Closed Rombur closed 4 years ago

Rombur commented 4 years ago

This PR works (I've only tried CPU) if I revert https://github.com/arborx/ArborX/pull/222 @masterleinad can you take a look at what we need to make sendAcrossNetwork work. One problem is obviously this but when I fixed it. I got the wrong results for the MeshFree tests.

Rombur commented 4 years ago

This is ready for review. The travis fails because of doxygen in arborx.

dalg24 commented 4 years ago

Can you comment to our relaxing the tolerance from 1e-14 to 1e-6

masterleinad commented 4 years ago

The travis fails because of doxygen in arborx.

We should probably just disable doxygen parsing arborx (either in this pull request or another one).

Rombur commented 4 years ago

Can you comment to our relaxing the tolerance from 1e-14 to 1e-6

Yeah, that's because we went from double to float in the point coordinates. I think that the important computations are done on double but we need to cast from float to double at some point. I looked at a few checks were I've relaxed the tolerance and it was clear that it was just due to a different round-off (value of 2.500001 instead of 2.5).

dalg24 commented 4 years ago

Can you comment to our relaxing the tolerance from 1e-14 to 1e-6

Yeah, that's because we went from double to float in the point coordinates. I think that the important computations are done on double but we need to cast from float to double at some point. I looked at a few checks were I've relaxed the tolerance and it was clear that it was just due to a different round-off (value of 2.500001 instead of 2.5).

Do you reckon we might have an issue here? We could recompute the distance in dtk in principle.

Rombur commented 4 years ago

Can you comment to our relaxing the tolerance from 1e-14 to 1e-6

Yeah, that's because we went from double to float in the point coordinates. I think that the important computations are done on double but we need to cast from float to double at some point. I looked at a few checks were I've relaxed the tolerance and it was clear that it was just due to a different round-off (value of 2.500001 instead of 2.5).

Do you reckon we might have an issue here? We could recompute the distance in dtk in principle.

In the current version of the code, this wouldn't help since all the points in dtk are using float now. I'll see how much work it is to use float only in arborx and double everywhere else.

Rombur commented 4 years ago

OK I've switched back to double and it works using a tolerance of 1e-14 instead of 1e-6. We get narrowing warning but that can be fixed in another PR

dalg24 commented 4 years ago

OK I've switched back to double and it works using a tolerance of 1e-14 instead of 1e-6. We get narrowing warning but that can be fixed in another PR

Great. Tanks for fixing this.