Various adjustments

[olesenm]

Hi @MakisH - just gave a test compilation with the current OpenFOAM develop branch (mostly wanted to see if we made any changes that broke compilation for you). Needed a few changes for clang (ambiguity with tmp -> Field etc).

While I was looking at the code, noticed a few places with copy instead of reference (copying the pressure field is the nastiest one).

Cleaned up some of the handling of adding in checkpoint fields to remove large chunks of near-duplication code. The bit with getObjectPtr may or may not be what you want (depending on which versions of OpenFOAM you are targeting). Can replace with the equivalent:

mesh_.db().getObjectPtr<volScalarField>(...) ->
&(const_cast<volScalarField>(mesh_.db().lookupObject<volScalarField(...))

A bit more typing, but it's wrapped in a macro anyhow.
Either way, for this type of code I think it is better to be passing through the pointer and let the called method determine its validity. This makes it easier to wrap up a find/add logic together.

[davidscn]

Awesome, thanks for this great contribution! My personal understanding of tmp in the OpeFOAM context was never good enough in order to apply it appropriately and I thought auto might allow me to get away with it, which seems to be wrong.

We had a few bug fixes within the last weeks and are now waiting for the upcoming OpenFOAM release in order to release the adapter alongside.

Now that you already touch the checkpointing allow me to ask a question: we have oftentimes the case that we reload the checkpoint after a single time step was computed. Does OpenFOAM store fields of the previous time step automatically? If it does, we could access these 'old' fields in order to reload the checkpointing instead of storing them explicitly and reloading them later on (would need to reload the old fields, though). This could save us a considerable amount of memory (namely all global fields at least once). I never really figured this out.

[olesenm]

(namely all global fields at least once). I never really figured this out.

Can't answer immediately, need a closer look at how/where it is being used. I suspect will also need a closer look at the registry flags to see what the adapter has done.

[MakisH]

Thanks a lot for the contributions and for the amazing care of checking for breaking changes! I have again a very good feeling about this OpenFOAM release.

Nothing of these changes should not work for past versions of OpenFOAM, right? Everything looks familiar.

By the way, a large part of this PR closes #55.

[davidscn]

I also resolved the merge conflict as it was more or less my change leading to the conflict.

[davidscn]

Can't answer immediately, need a closer look at how/where it is being used. I suspect will also need a closer look at the registry flags to see what the adapter has done.

Merging here, but want still want to mention that I'm interest in this.

[olesenm]

Merging here, but want still want to mention that I'm interest in this.

Sure, but deserves a separate issue.

[davidscn]

Absolutely.

[davidscn]

Opened in #210

[MakisH]

An update on the compatibility with other versions:

• All recent .com version (v1912-v2112) built and ran successfully in our new GitHub Actions workflow to build with any version.
• All .org versions (4-8) complained at the exact point that @olesenm mentioned above

In the OpenFOAM4 branch (on which every other .org branch is based upon), I applied the following patch to make it work:

-    for (const word& obj : mesh_.sortedNames<GeomField>())               \
-    {                                                                    \
-        addCheckpointField(mesh_.thisDb().getObjectPtr<GeomField>(obj)); \
+    for (const word& obj : mesh_.lookupClass<GeomField>().toc())                                    \
+    {                                                                                               \
+        addCheckpointField(&(const_cast<GeomField&>(mesh_.thisDb().lookupObject<GeomField>(obj)))); \

Note that I reverted the sortedNames back to the toc, as I could not find a templated version of sortedNames in OpenFOAM4.

I hope I did not overlook/misuse anything here. I sometimes get confused in these long chains of lookups and type conversions (loving auto and C++17 automatic type deduction in templates).

[olesenm]

@MakisH Should at least use sortedToc() instead of toc() for overall consistency

[MakisH]

@MakisH Should at least use sortedToc() instead of toc() for overall consistency

Good point, thank you. I applied this patch and rebased again.