search

adamantine-sim / adamantine

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

Changed eps in timestepping #254

Closed AshGannon closed 5 months ago

AshGannon commented 5 months ago

We use an epsilon to get the "expected" behavior when the deposition time and the time match should match exactly but don't because of floating point accuracy. I changed eps to be computed using the deposition start time and error between the time step and activation time.

AshGannon commented 5 months ago

I only ended up changing the denominator of eps.

stvdwtt commented 5 months ago

@AshGannon do you have a sense for when the epsilon is too small and when it isn't? Are we safe hard-coding it like you have, or should it depend on something problem specific?

Rombur commented 5 months ago

The tolerance already depends on the problem through the time step. It's obviously not ideal to have a "magic" number in the code but when dealing with round-off you usually don't have a choice.

The PR is not passing the CI but that's because we need to merge #253 first.

AshGannon commented 5 months ago

@AshGannon do you have a sense for when the epsilon is too small and when it isn't? Are we safe hard-coding it like you have, or should it depend on something problem specific?

Bruno's answer is better.

Bruno mentioned to me that he originally chose 1e12 because you can expect an accuracy of about 12 digits with double precision. He also uses this eps for power and time in deposition_along_scan_path. In my hourglass simulation, the dwell time is set to 0, so my deposition_time.begin() happened to be 1e-12. The deposition_time was also not set. Since double eps = (*deposition_times.begin())/abs(activation_time_end - time); worked in this example, double eps = 1e-12/(time - time_old) might work generally. It's almost the same as dt/1e12, but you accumulate error in dt as time goes on.

stvdwtt commented 5 months ago

Retest this please

AshGannon commented 5 months ago

@stvdwtt I'm not sure what you mean. Retest the 1e-10? Or the dt/time-time_old? I ran the latter over the weekend, the mesh fully activated and the results were very close at the times I checked (just spot checked). Are you seeing something odd with the 1e-10?

stvdwtt commented 5 months ago

@AshGannon Sorry! That wasn't directed at you -- that's the command for the continuous integration tests to run again. I just merged in Bruno's PR and my understanding is that the tests in this PR should pass now. Assuming they do, we'll merge this.