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

Build Script Option for Different version of WWM #4

Closed
jamal919 opened this issue Aug 21, 2019 · 7 comments
Closed

Build Script Option for Different version of WWM #4

jamal919 opened this issue Aug 21, 2019 · 7 comments

Comments

@jamal919
Copy link
Member

Since the sourcecode of SCHISM has been moved from SVN to Git, the several other branches that were made in the course of last two months has not been shifted here in Git. One of the possible consequence of this is probably the development branches of different modules - WWM is one of such example.

One version of WWM that I am particularly interested in is the La Rochelle branch. Few months ago, the La Rochelle branch was added officially to a separate SVN branch, which continued developement from the original WWM.3012 (with same folder name). Another version is the official version (with folder name WWM).

The two code folder for one module may seem problematic to people and I hope that in the future these two codes get merged to one place implementing on something that the user community agrees and thoroughly tested. Until such time comes, is it possible to add an option in the build script to define which WWM version is to be built? The two version I have in mind are La Rochelle Branch (WWM.3012) and the Trunk (WWM).

@huyquangtranaus
Copy link
Contributor

Hi @jamal919 ,

I believe you can do it easily. In the src/Makefile, you just change:
#Search path
VPATH = Core/ Driver/ Hydro/ EcoSim/ COSINE/ Sediment/ Sed2d/
WWMIII/ ICM/ TIMOR/ FIB/ Ice/ Fabm/
by:
#Search path
VPATH = Core/ Driver/ Hydro/ EcoSim/ COSINE/ Sediment/ Sed2d/
WWMIII.R3012/ ICM/ TIMOR/ FIB/ Ice/ Fabm/

or to whatever you like.

@jamal919
Copy link
Member Author

Thanks @huyquangt, I understood the change you suggested. The problem is WWM.3012 bundled with current releases does not work directly with this change, because the files to be compiled are not same for the two releases of WWM.

@jreniel
Copy link
Member

jreniel commented Aug 21, 2019

Drifting here a bit on the philosophical side... The WWM branch you are referring to is not yet official, correct? The SCHISM master branch should only support official releases of the module. If you are interested in support for other branches in other modules (which are to be treated as forks in git) what you could do is fork the SCHISM repo and modify it so this branch is supported. In the future, when the module branch is integrated into the main repo, then we can merge your branch to the master to support officially these new features. In general, the master branch should only support what is tested. Everything else is up to us, the developers, to work out on our forks so we can prepare the modifications needed for future merging. @jamal919 @huyquangt @josephzhang8 @water-e Please take note of this thread and comment, because this is precisely the kind of situation that needs to be communicated so we can all have effective collaboration. Please post your thoughts and opinions.

@jreniel
Copy link
Member

jreniel commented Aug 21, 2019

@jamal919
Alternatively, you can create a branch on the main schism repo, but as long as the La Rochelle branch is not merged to the WWM trunk, we are dealing with what is to be treated as a completely separate and different software, and we are simply inviting divergence. This is why I'd recommend to work on this as a fork, because technically, the La Rochelle branch as of now is a WWM fork, and therefore a completely different software. We cannot think of this as interchangeable if the branch in question is not backwards compatible.

@jamal919
Copy link
Member Author

@jreniel, thanks for your thoughts. I can not agree more with your comments. And yes, as far as I have understood the La Rochelle version is not official, but added as a separate branch to facilitate the continuation of the development they are pursusing by @josephzhang8.

Ideal scenario is of course to have a single implementation of a single module, not two as it appear now. Since the user/developer community of WWM-SCHISM can safely be assumed to be smaller than the SCHISM-Core this issue will probably persists for some more time. Any idea or roadmap for unification is very much appreciated, of course.

@jreniel
Copy link
Member

jreniel commented Aug 21, 2019

@jamal919 I'm glad to know that we are on agreement. Because we're just starting up the git, and I am a newcomer to the SCHISM community, I was a bit afraid of what would be the response. But I am glad that it seems the ship is steering well. But back to the main discussion (the following are just ideas, don't take them as absolutes)...

First I have to ask, are there any plans to integrate the La Rochelle branch into the trunk at some point? If the answer is yes, then I suppose compilation steps will change, so this is a semi major version change of schism, and you may start tracking this via a feature branch on the main repo (if collaborative tracking is expected), or a fork (if you're flying this one solo). Then when the the La Rochelle branch gets merged, we can do a PR to a test branch and finally merge the test branch to the master, along with a semi major version number increase and new tag.

If the La Rochelle branch is expected to diverge, then perhaps the schism master should not consider it at all. Any schism version interested in using the La Rochelle version should fork schism and name it schism_la_rochelle or something else. But the point is that La Rochelle, as of now, is a fork, until it becomes part of the trunk.

That being said, I would love to see all these modules be brought to git. We can work with SVN trunks, but git based development would be beneficial I think for everyone. In any case, I'm available to help out with those migrations as well if the parties involved are interested.

@jreniel
Copy link
Member

jreniel commented Aug 23, 2019

Closing issue since it seems we are on strategic agreement.

@jreniel jreniel closed this as completed Aug 23, 2019
josephzhang8 added a commit that referenced this issue Nov 16, 2020
* update notes

* veg_icm frame

* boxes left for light/inundation stress + init

* backup before adding dry_sediment_diagenesis

* sediment diagenesis under dry condition

* backup for optional mortality process

* veg light interactions + renew sav whole volume unit

* round for check; back up for dynamic linkage

* non-dynamic feedback set for multi-veg condition

* backup before going through diff and adding debugging outputs/msg

* backup test 1

* backup before removing mht etc; replace morris formula with two linear equaitons since it is universal for all locations

* benchmark #2, to add veg-related process on nutrient dynamics; leave full dynamics loop apart after benchmark #3 with both veg+sav

* benchmark#3, to fix temperal variation + spatial distribution; fixed SAV unit bug for nutrient dynamic processes + allow co-occupation

* reset CMakeLists in WWMIII_Sept2020

* backup for benchmark #3; to modify parameters + add full feedback loop

* benchmark #4, renewed nutrient recycle and tested equilibrium

* add vegetation production; modify hcan calc

Co-authored-by: Nicole Cai <ncai@vims.edu>
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

No branches or pull requests

3 participants