RangeShifter / RScore

RangeShifter's core simulation code
GNU General Public License v3.0
0 stars 0 forks source link

Compare release-batch with (release-r) develop #2

Closed TheoPannetier closed 11 months ago

TheoPannetier commented 12 months ago

I have created develop and the new master from release-r (mistake?), but now realise that it differs from release-batch in non-trivial ways, see the merge conflict section of this PR. @JetteReeg I'm not sure which version should be kept as the main version of the code? Or de wo have to maintain both separately? I'm not a fan of the idea of keeping two concurrent master branches, I feel this would partly defeat the purpose of using a subtree. Could it be a solution to use an #ifdef macro here?

JetteReeg commented 12 months ago

Hi Theo, when creating the release versions, I ued the unifdef function, that Anne Malchow (who developed the R package and I think also cleaned the batch code back then) provided. Batch and R version both need different Macro settings. Batch version: unifdef_RS2.txt R version: unifdef_Rpkg.txt That's why there need to be 2 release versions, if we want to have the release version cleaned from all macros. I assume we could have a release branch, where only the macros necessary for batch/r version are included?

TheoPannetier commented 12 months ago

Thanks for sharing the unifdef scripts!

That's why there need to be 2 release versions, if we want to have the release version cleaned from all macros. I assume we could have a release branch, where only the macros necessary for batch/r version are included?

I would actually be in favour of maintaining a single release branch (master) in RScore if possible, to minimise the risk of confusion over which branch to use for users, and reduce the chance of merge conflicts for us. Do you reckon it would be achievable, by using some #if macros? For example, resolving the conflict on L460 of Community.cpp:

<<<<<<< release-batch
void Community::dispersal(short landIx)
=======
void Community::dispersal(short landIx,short nextseason)
>>>>>>> develop

with

#if RS_RCPP
void Community::dispersal(short landIx,short nextseason)
#else 
void Community::dispersal(short landIx)
#endif // RS_RCPP
>>>>>>> develop

(not sure whether RS_RCPP is the correct one to use for the R compilation, but you get the idea?)

So we'd keep all the #if macros necessary for any of the interfaces (RS_RCPP, RS_BATCH, RS_GUI, LINUX_CLUSTER) on the single release (master) and develop branches. All of the optional #if macros (SEASONAL, RS_CONTAIN, GOBYMODEL etc.) would be removed and live on a separate (e.g. macros) branch, at least until we decide that they should be fully integrated within the core code.

From https://github.com/JetteReeg/RangeShifter_restruct/pull/2 :

So your idea would be, to keep all the older versions on the one hand inside an overall 'macros' branch as well as in specific development branches, e.g. RS_CONTAIN. Just so we have them centrally stored in the same repository.

Those would be mutually exclusive alternatives, actually. So either we would:

  1. Keep all optional macros that we don't actively maintain (SEASONAL, RS_CONTAIN, GOBYMODEL etc.) together on a macros branch, or
  2. Each one of these options lives on its own branch.

In both cases, the macro branches are downstream from master and develop, so that when we implement something new in Rangeshifter, we can simply merge master/develop into the macro branch to update the optional versions as well.

But whenever we would now stark working on new features or would like to modify the official version, we would create a new branch from the developement? Is that correct?

Indeed! so for example when I start working on the new genetics bit, I will do something like this:

git checkout develop
git branch new-genetics
git checkout new-genetics
// ... work on new genetics

// every now and then to update new-genetics with new changes other people have put on develop
git merge develop

When I'm done and ready to put the new genetics in rangeshifter, I would open a pull request to merge new-genetics into develop, or directly git merge new-genetics from develop if we don't use PRs. When we're ready to release, someone merges develop into master (with or without a pull request).

JetteReeg commented 11 months ago

Hi Theo, I ran the unifdef tool on the macros branch and kept only those macros, which differed between the batch release and R release:

unifdef_Rpkg_batchRS.txt

I probably need to recheck for some 'OR' conditions (e.g. the example you mentioned, it still says if SEASON || RS_RCPP).

We still need to test, if both, Batch and R, work with that code.

The rest sounds nice to me. Let's discuss later with Greta what would be the best way to deal with the other macros. Would be also good to know, if people are currently working with some of these versions.

TheoPannetier commented 11 months ago

Closing following our chat last Thursday - now using release-common as the basis for develop and master