precice / calculix-adapter

preCICE-adapter for the CSM code CalculiX
GNU General Public License v3.0
52 stars 20 forks source link

CalculiX Restart Errors #67

Open vollmerb opened 3 years ago

vollmerb commented 3 years ago

I have found that CalculiX contains an error in dynamic mode on restart where the restarted results will not match identically to a non-restarted run. This is fundamentally a CalculiX issue due to a prediction step involving accelerations, which are not saved in the restart files.

With these fixes, the coupled restarts are identical to a non-restart (in my tests) to the ~14th digit, as opposed to ~5th digit. The lingering error is likely due to a final results evaluation on exit, but is sufficiently close to double precision in my estimation.

The issue here is the fixes require the updated CalculiX repository and the precice adapter. The CalculiX github repository appears unofficial, and I have mentioned my restart issue on the discourse group (https://calculix.discourse.group/t/restart-failures-dynamic/623).

Does anyone have suggestions/ideas on how to incorporate this fix more permanently?

IshaanDesai commented 3 years ago

Hi @vollmerb

firstly it is great that you noticed the difference in restart results for the dynamic mode, and that you already found a fix. From the perspective of the CalculiX adapter the problem seems to be that you have fixed this in a later version of CalculiX and the adapter is not up-to-date, is this right? Or did I misunderstand something? Which CalculiX version does your CalculiX fork and your adapter fork work with?

You can open a pull request in this repository so that we can review the changes and merge them.

vollmerb commented 3 years ago

Hi @IshaanDesai,

The fixes I made are in Calculix v2.16 and the most recent precice calculix-adapter (so everything is using the most recent versions). So from the perspective of the adapter, the fixes will only work properly if Calculix is also updated. Perhaps we could replace the Calculix restart files from the adapter build (much like how ccx_2.16.c is used).

As far as a pull request, I have not tested my changes beyond a serial dynamic restart, and suspect issues for parallel/other operating modes. My fork also contains changes specific to my setup. I plan to eventually get around to testing the fixes more thoroughly, but for now they are just a quick prototype.

IshaanDesai commented 3 years ago

Hi @vollmerb,

Okay now I understand the situation better. I would then suggest to wait for your changes to be added to the CalculiX repository and then we can correspondingly update the adapter too.

When you are done testing your changes, please let us know so that we can start discussing in a pull request on how to merge them into the adapter.

uekerman commented 2 years ago

We should check whether the fix is now included in CalculiX and if yes, we could probably close this issue.

KyleDavisSA commented 2 years ago

It does not appear that Calculix has implemented these changes. Creating a PR for this would currently break the adapter.