adamantine-sim / adamantine

Software to simulate heat transfer for additive manufacturing
https://adamantine-sim.github.io/adamantine/
Other
32 stars 9 forks source link

Fix experimental data with FE_Nothing, refactor experimental data interface #219

Closed stvdwtt closed 1 year ago

stvdwtt commented 1 year ago

Partially address #181.

This PR makes the extraction of the DOFs with experimental data work when some elements are FE_Nothing. It also refactors the interface between the functions in experimental_data and the DataAssimilator. The pair of vectors that map between the experimental observation indices and the DOF indices is now calculated in experimental_data, instead of an intermediate being passed through run_ensemble. The updated interface makes the "dummy" temperature field in run_ensemble unnecessary.

The test for experimental data with FE_Nothing is real IR data (albeit not particularly well calibrated IR data). The test has a runtime on the longer side, which is due to the time calculating the intersections with the mesh. The DOF has limited impact on the runtime, it scales as the number of IR pixels. We could consider a smaller experimental data file. @Rombur what do you think?

stvdwtt commented 1 year ago

CI

The CI failed, I'm looking into it.

Rombur commented 1 year ago

In ArborX, the rays are defined by an origin and a direction. The direction is normalized. The error message says that the norm of the direction is not greater than zero

Rombur commented 1 year ago

The test has a runtime on the longer side, which is due to the time calculating the intersections with the mesh.

Is it the time spent in ArborX or the time spent on compute the exact intersection in a cell?

stvdwtt commented 1 year ago

The error message was related to improper parsing of the header for the CSV file. I modified the README to explicitly state that the file should have a header and now the parser skips the header.

Now the test is running substantially faster (1.33 s on my laptop, 22.91 s on a cloud VM). Maybe the slow performance was related to the uncaught vector magnitude error.

stvdwtt commented 1 year ago

New CI

stvdwtt commented 1 year ago

New CI

Rombur commented 1 year ago

I've restarted the PR 3 times but it keeps failing the CI. I think it's related to https://github.com/adamantine-sim/adamantine/pull/218#issuecomment-1428673284 I found the problem using valgrind outside of CTest.

stvdwtt commented 1 year ago

@Rombur are you comfortable merging this PR since the test failures are (in all likelihood) unrelated to these changes?

Confusingly, some of the errors are from CUDA/test_thermal_operator_1 and some are CPU-on-device/'test_integration_thermoelastic_1`.

Rombur commented 1 year ago

Let's merge this. I'll fix the other bug in a different PR