Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version fusion #128

Closed
wants to merge 3 commits into from
Closed

Version fusion #128

wants to merge 3 commits into from

Conversation

MakisH
Copy link
Member

@MakisH MakisH commented May 19, 2020

This pull request isolates version-specific changes into separate files included at compile-time (we could even say "configuration-time"), trying to unify all the different variants of the adapter we are currently offering as branches. See #32 for a discussion of the problem (closes #32).

Motivation:

  • Currently, with every version-specific branch we apply short patches.
  • OpenFOAM already splits code into parts that it includes in the middle of the implementation using #include "file.H".

Idea:

  • Put the parts that are version-specific into a separate directory variants/<variant>/version-specific/.
  • Include them in the middle of the implementation, e.g. as #include "version-specific/file.H".
  • Leave a comment with a concrete implementation example for readability (they must be short anyway).
  • As <variant> we always take the $WM_PROJECT_VERSION. We only treat it as a string as it varies a lot among versions.
  • We need a directory for every version. We define the files in the oldest supported version and create links for newer versions (to the complete directory and/or individual files).

Pros & Cons of this approach:

  • (+) No need for the user to change branches, always have the latest changes in all variants
  • (+) Easily extensible: Just find the closest version and copy the directory or make a link
    • Supporting a new version should usually mean "create a link to the previous version".
  • (+) Avoids duplication: Only define the patch in one file, link from other versions
  • (+/-) Clearly define which versions we support
  • (-) For a new version we always need a new directory/link --> users should get the latest adapter or patch the change
  • (-) To fix one version in a new location we need to update all the variants.
  • (-) Reduced readability/refactoring capability
    • But OpenFOAM itself already uses such a preprocessor-based approach
    • and we keep a comment of the implementation
    • we will have an issue if fundamental functions we use all the time start changing (e.g. older .boundaryField() and .boundaryFieldRef().

Todo:

Before merging, check that the final version builds with:

  • OpenFOAM 4.1
  • OpenFOAM 5.x
  • OpenFOAM 6
  • OpenFOAM 7
  • OpenFOAM-dev
  • OpenFOAM v1706
  • OpenFOAM v1712
  • OpenFOAM v1806
  • OpenFOAM v1812
  • OpenFOAM v1906
  • OpenFOAM v1912
  • OpenFOAM v2006

External updates:

  • Check/update tutorial scripts (especially FSI tutorials)
  • Update wiki page Notes on OpenFOAM
  • Update system tests

Possible next steps:

  • Automatically decide between pimpleFoam and pimpleDyMFoam in the tutorial scripts. Maybe simply apply a special case for the oldest supported versions.

@MakisH MakisH changed the base branch from master to develop May 19, 2020 09:09
@MakisH
Copy link
Member Author

MakisH commented Apr 29, 2021

Since we decided (#52) to keep support for the Foundation branches and release archives for each range of supported versions, I am closing this for now. It is an idea that we may revive in the future, though, so I would like to keep the branch.

@MakisH MakisH closed this Apr 29, 2021
@MakisH MakisH deleted the version-fusion branch November 22, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant