Coder Social home page Coder Social logo

Comments (37)

water-e avatar water-e commented on June 27, 2024 2

OK, I just confirmed on my copy that reordering the tags works fine when you check out the v5.10.0 tag and use gen_version.py. I'll push it soon.

Before that, any opinion what we want pschism -v to say when someone builds master? Right now it lists 5.9mod because 5.9 is the last tag in that lineage. That is not accurate for post-5.10 work. I could tag something a ways back in the master lineage after the 5.10 release branch and call it 5.11-master. That will get rid of the 5.9 stuff and I think gen_version will barf and cause pschism -v to say something like "develop-1913sdats" where the latter part is the hash. That is good, yes? We don't really want a semantic version number off random places on master anyhow. Anyone have an opinion?

By the way the release branch v5.10 was probably always timed correctly, I was just misreading the location of the HEAD as the location of the branch point. The only problem was the ordering of the noaa and semantic tags.

from schism.

jamal919 avatar jamal919 commented on June 27, 2024 2

@hot007 I am suspecting that the executable name is hard defined in the Make.defs.local file, see variable EXEC.

With the compiled executable if you dopschism_5.8.0_HYDRO_VL -v, it will show that the version of the code is v5.10.0

from schism.

water-e avatar water-e commented on June 27, 2024 1

OK, I see two things here that might be worth looking into.

First, it appears that v5.10.0 and stofs3d-atl.v.1.1.0 tags both point to the same thing. This is something that is known to confuse git describe.

The second issue is that the ordering is weird. I'm less confident, so forgive me if I'm wrong, but it seems that the release branch v5.10 is derived from the tag v5.10.0. If so, that would be the wrong way to go about things. You want the official release branch v5.10 before you start tagging specific versions like v5.10.0. You could always tag something pre-v5.10 if you want to tag the state before the release branch.

I think we should address both these issues. If we don't care about stofs3d-atl.v.1.1.0 removing it is probably one-stop shopping for fixing the first problem. I doubt this is the case though -- it seems to have some meaning to NOAA. Anyhow, I think we may see the double tagging get revenge again later -- as long as we are doing this double tag there will be some risk that folks will forget that the order matters.

Dealing with the ambiguous tag doesn't address the second problem of things being out of order. if I'm reading the diagrams correctly the release branch v5.10 at commit 53fbf7 has the tag v5.10.0 as its ancestor. To be specific the lineage from the release branch (53fbf7) backward is 3cf45d, 4ddd680, fd48b3 and finally 1e0247 which is the target of the v5.10/stofs tags . All these intermediate commits have messages involving documentation and affect yaml files, so I assume making a few changes in where the tag is pointing to after the branch would only improve things and would not impact the state of the numerical code.

One way to fix this in GitHub this:

  1. Delete both the stofs3d and v5.10.0 tags.
  2. Re-create the stofs tag after v5.10 branch.
  3. Make a trivial commit
  4. Add the v5.10.0 tag.

The trivial commit is there to make the two tags non-duplicate. It may be that it is enough that the v5.10.0 is the most recent tag, so a slight variant would be to try skipping the trivial commit and just reordering the commits as shown.

And heck, if just changing the order is going to work then we could probably do something that doesn't change the state of 5.10.0 at all even down to the documentation:

  1. delete the tags and the release branch
  2. reintroduce all three where the tags are now, in the order: v5.10, stofs3d-atl.v1.1.0, v5.10.0.
    That would leave Jamal's documentation changes after that commit stranded. We could add a v5.10.1 at the end of the line (where v5.10 is now) to pick up the extra documentation. This kind of sounds like overkill, though. I think most users would be happy if the v5.10.0 code didn't call itself 5.9.

I'm happy to try any of these. I'm fine implementing it, but I'm awaiting your feedback. I think we should wait until this is straightened out before going to Claire's addition.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024 1

from schism.

jamal919 avatar jamal919 commented on June 27, 2024 1

Is there a way I can change that Make.defs.local file to harvest the version value from git in the Make.defs files? Sorry that's probably a silly question but maybe we need to pass in an env var or something for that?

I guess you can simply change the EXEC value to pschism-`git describe`? That will add the output from git describe to the file name.

FYI:

  1. I have not tested it, but should work.
  2. File name may become quite long

from schism.

jamal919 avatar jamal919 commented on June 27, 2024 1

It is indeed clear from all the discussion/examples that using git describe directly is not a good idea. Personally, I always use a static name like pschism-VL_MOD1_MOD2, and check the version from my submission scripts to see if I am indeed running the right executable. But at the same time, for my need I do not change version that often.

If I need to keep a versioned executable in make, for now my solution is rather band-aid approach (similar to above) using git-rev-parse, e.g., EXEC=pschism_`git rev-parse --short HEAD``git diff --quiet || echo '-dirty'`

I like the idea of tackling the version number issue with gen_version.py. May be if gen_version.py script echos the version number, it can be directly used in initialising the EXEC variable? Not sure it is an uniform approach between make and cmake though.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024 1

from schism.

water-e avatar water-e commented on June 27, 2024

I am able to reproduce the behavior on my system.

The proposed fix isn't safe. For instance, you can check out the code and type 'git checkout v5.8.0' and git tag will show v5.10.0 as the maximum "v" version. It will also report v5.10.0 on the trunk as well in between versions. In short, the "latest tag" can be different from what you want. If we are disciplined about release branches, though, it will not be.

v5.10.0 was originally built atop a release branch but it has been deleted and that is a misunderstanding about best practices. In fact, we don't really have a reason for calling a tag v5.10.0 at all if we have no v5.10 release branch to continue the lineage and create a v5.10.1 bug fix without mixing in new development. I believe that maintaining the release branch will make it simpler to trace closest reachable tags. Obviously, this works sometimes -- v5.9 and v5.8.0 don't have this issue and it is probably the confounding tag you mentioned that causes the problem.

This is a bit of an unfortunate situation, and I think the best solution is fix it with version control. Fortunately it is pretty easy to go back into the past. I am pretty sure we can either go back and recreate the v5.10 branch and v5.10.0 tag, deleting it and replacing it (the code will be identical) or we could move on to v5.11.0 if the code isn't in upheaval.

from schism.

hot007 avatar hot007 commented on June 27, 2024

I notice the output of make still tends to be things like pschism_5.8.0_HYDRO_VL instead of 5.x where x is derived from the checkout tag, be good to sort that at the same time :)

from schism.

water-e avatar water-e commented on June 27, 2024

Claire, OK we can include that. You are using GNU make? It might not matter ... I suppose if this is broken one place it is broken several places. Joseph has pointed out by email that v5.10 branch is still in there -- I was looking at a truncated list. But we still need to figure out why git describe doesn't work. I'll look at it in git kraken which has good diagrams that might reveal something.

from schism.

jamal919 avatar jamal919 commented on June 27, 2024

Thanks @water-e for debugging it. If it fixes, that will be of great help. BTW, I fully agree with your conclusion that on master branch the temporary fix I proposed is not very robust. However, it should work in individual release branch (tag and branch). In any case, the most important thing is probably to get a consistent version numbering.

A related topic to gen_version.py script - would it be worthwhile to replace gen_version.py with bash/shell script to reduce the dependency on python for compilation?

I think we should address both these issues. If we don't care about stofs3d-atl.v.1.1.0 removing it is probably one-stop shopping for fixing the first problem. I doubt this is the case though -- it seems to have some meaning to NOAA. Anyhow, I think we may see the double tagging get revenge again later -- as long as we are doing this double tag there will be some risk that folks will forget that the order matters.

Pardon me if I am getting on the wrong foot, but tagging production code of NOAA and standard schism release in one single repository will probably only increase confusion (for both NOAA side and other user community side). It appears to me that a good strategy for NOAA would probably be to fork the schism-dev/schism to NOAA-XYZ*/schism, and tag/branch as necessary for production (with periodic syncing with the upstream)?

*XYZ being the associated division of NOAA, at least that seems to be the naming scheme of the github account of various divisions

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

water-e avatar water-e commented on June 27, 2024

Jamal,
We'll get the consistency one way or another. I don't think 'git tag' is that -- it would change the number assigned in a stable branch as things are added. The NOAA idea of retagging is awkward, but I blame git describe and the inherent difficulty of getting the right info as much as anything. We'll see how the NOAA folks and Joseph respond.

As to the suggestion about phasing it out this would be a community decision. I vote no with both hands and feet. I (and several others) joined in a discussion about switching python on some of the scripts.

I find python much more self-documenting than bash and so did a few others and I've always found python available, albeit out of date, even on esoteric machines (defunct Cray etc). Also this script is meant to negotiate situations like where there is no git or the user has their own system and that isn't going to be a one-line bash script. That is probably not functioning right at the moment (Claire's issue) but it is a small matter to repair it. So .... is this really a concrete problem or a musing?

There is another cost to me. In our group, we maintain a windows compilation (5.10 due soon) that is reasonably performant and that we use for classes. It seems silly to have cmake plumbed out all the way to this point and then use bash. We could possibly use raw cmake for this, but gen_version as a prerequisite would stay for windows and then what would we have achieved? It is also a fair amount of logic for cmake.

gen_version.py should work on 2.x or 3.x and if it turns out that isn't true I'll fix it so it does. If folks are working widely on machines that don't have python please let me know.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

jamal919 avatar jamal919 commented on June 27, 2024

We'll get the consistency one way or another. I don't think 'git tag' is that -- it would change the number assigned in a stable branch as things are added. The NOAA idea of retagging is awkward, but I blame git describe and the inherent difficulty of getting the right info as much as anything. We'll see how the NOAA folks and Joseph respond.

I agree, git tag is not a solution, but a band-aid :)

As to the suggestion about phasing it out this would be a community decision. I vote no with both hands and feet. I (and several others) joined in a discussion about switching python on some of the scripts.

I find python much more self-documenting than bash and so did a few others and I've always found python available, albeit out of date, even on esoteric machines (defunct Cray etc). Also this script is meant to negotiate situations like where there is no git or the user has their own system and that isn't going to be a one-line bash script. That is probably not functioning right at the moment (Claire's issue) but it is a small matter to repair it. So .... is this really a concrete problem or a musing?

There is another cost to me. In our group, we maintain a windows compilation (5.10 due soon) that is reasonably performant and that we use for classes. It seems silly to have cmake plumbed out all the way to this point and then use bash. We could possibly use raw cmake for this, but gen_version as a prerequisite would stay for windows and then what would we have achieved? It is also a fair amount of logic for cmake.

gen_version.py should work on 2.x or 3.x and if it turns out that isn't true I'll fix it so it does. If folks are working widely on machines that don't have python please let me know.

Thanks for the long reasoning :). To begin, my vote is also for python and I was not suggesting for a change. Personally, I also do everything through python. It was rather just a curious question, and the root of it lies to a time when I was browsing for ways to extract the version number for a personal project. I ended up using python, BTW. Like everything there is multiple packages, and I was lazy to write a gen_version.py. But in the searching process, I found that git itself use a bash script - https://github.com/git/git/blob/master/GIT-VERSION-GEN - hence my question. The change of the gen_version script is clearly not worthwhile, however this exchange certainly was.

from schism.

water-e avatar water-e commented on June 27, 2024

OK, Joseph I think my solution is going to work. I think it can be deleted if it doesn't. Tags and branches can always be recreated as necessary. Shall I proceed? I think I can do it locally first anyhow.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

water-e avatar water-e commented on June 27, 2024

I can make them point to the same hash they did before, both the same. However in that case I have to delete and recreate 5.10.0 on the newer side of stofs*. Git seems to regard the later creation time as "most recent in lineage" if the the hashes are a tie. Based on a sample of one.

from schism.

water-e avatar water-e commented on June 27, 2024

Here is the change. Putting it here because no other real record of it. It works on a round trip for me. @hot007 Can you show me what remains for you after you git fetch and build. I'd need to know what your executable gets named, whether you are cmake or GNU make. Thanks for bearing with me ... this kind of surgery makes me (and I'm sure Joseph) nervous.

# Delete the tags that need swapping  
git tag -d stofs3d-atl.v1.1.0
git push --delete origin stofs3d-atl.v1.1.0 
git tag -d v5.10.0
git push --delete origin v5.10.0
# Retag in NOAA then semantic order, pushing one at a time due to supersticious nature
git tag -a stofs3d-atl.v1.1.0 1e0247 -m"Revised stofs3d-atl.v1.1.0 tag to precede 5.10.0 in hopes this will be read cleanly by git describe and hence fix gen_version.py"
git push origin stofs3d-atl.v1.1.0
git tag -a v5.10.0 1e0247 -m"Revised v5.10.0 after NOAA tag in hopes this will be read cleanly by git describe and hence fix gen_version.py"
git push origin v5.10.0
# Tag master sligthly after 5.10 release branch. gen_version.py won't actually know what to do with this but
# when it breaks it will produce a good hash-based version
git tag -a master-post-5.10 -m"Tagged a commit soon after 5.10 so that it will be found by gen_version.py" 1c62c24a7b5dd7b82ec830445c422e70ab1664dc
git push origin master-post-5.10

from schism.

hot007 avatar hot007 commented on June 27, 2024

Yep okay, I'm using gnu make, my process is

git clone https://github.com/schism-dev/schism.git
cd schism
git checkout tags/v5.10.0
cd mk
# copy in Make.defs.gadi from host
ln -s Make.defs.gadi Make.defs.local
cd ..
module purge
module load intel-compiler/2019.3.199
module load openmpi/3.1.4
module load netcdf/4.7.3
module load python2-as-python
module list
# assumes we are in the directory that contains the source code
# e.g. git clone https://github.com/schism-dev/schism.git -> schism
cd src
make clean
make psc

Output from make includes

 SCHISM version:  v5.10.0
 GIT commit       1e02474

but output executable is, unfortunately, still pschism_5.8.0_HYDRO_VL
I've just been manually renaming the executable but this seems like a relevant time to bring it up here! (v5.9 was the same).

from schism.

hot007 avatar hot007 commented on June 27, 2024

See also #40

from schism.

hot007 avatar hot007 commented on June 27, 2024

Ah, you are right! We had EXEC= set to include 5.8.0. Is there a way I can change that Make.defs.local file to harvest the version value from git in the Make.defs files? Sorry that's probably a silly question but maybe we need to pass in an env var or something for that?
How are other people handling this or do you just not show the version in the executable name and rely on -v? That is also an option of course!

from schism.

water-e avatar water-e commented on June 27, 2024

Depends on what you want the behavior to be when git describe produces something funny. I'm not speaking of the latest problem, more like ... what do you want on master which should probably be the current hash? As before, Jamal, raw calls to git describe neglects the corner cases that occur when there is no tag or the result is funky. Ideally, you want master to produce the hash possibly with 'mod' and you want the user to be able to override.

One option that would be uniform across GNU and CMake would be to tackle these issues as they arise in gen_version.py and then store its output as an environment variable ... and then incorporate that in the executable name in the respective make systems.

It has been a while since I've used the GNU Makefile (I played a role in creating it many years ago but I find it horrifying compared to cmake), so I kind of dread dusting that off, but I can support the python part for the executable (it is just one added line setting os.env). I think we probably haven't seen the end of the corner cases.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

water-e avatar water-e commented on June 27, 2024

from schism.

brey avatar brey commented on June 27, 2024

Dear All, may I re-iterate issue #50 and ask for a native __version function that returns the current tag without the git dependency.

from schism.

water-e avatar water-e commented on June 27, 2024

from schism.

brey avatar brey commented on June 27, 2024

I understand there are two different use cases. I understand the need to have the git versioning working. Maybe this can be automated with something like dunamai?

However the semantic versioning is important for the imminent conda release #13. In addition, the text return of "schism -v" should indeed be the last commit before a release which should coincide with a set of tests working properly and a tag. Currently, there are tags but no releases after 5.9. For distributing the code with coda the git versioning doesn't work.

In fact, I would expect that each release has its own branch for future hot fixes. Also tags are not immutable and shouldn't be used for versioning.

I am open to suggestions on how to best deploy semantic versioning and release management but I think, even if automated, the release should be deliberate and managed while tagging can be more liberal.

Also of note, we are exploring SCHISM as an operational tool. In this context, changing versions is a more elaborate exercise which is not easy to undergo often. The repo management should be able to handle long term support of such processes better.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

brey avatar brey commented on June 27, 2024

You are right the branches are there. I was mislead by the fact that there are no new Github releases (see on the right side of the Github page).

from schism.

pmav99 avatar pmav99 commented on June 27, 2024

@josephzhang8 @water-e
A bit more context might make our use-case/intentions clearer

  1. A conda package installs an executable + necessary libraries. Whoever installs the schism conda package does not need to know anything about compiling schism and/or manipulating the version on runtime. Furthermore, it would not be uncommon for such a user to have multiple versions of the package installed (in different envrionments). All that user expects is that schism -v/--version will print something sensible like make -v/--version, gcc -v/--version and cmake --version do. The output could contain additional information that has to do with the compilation/configuration options, pretty much like gcc -v does, but you could also have a dedicated command for that (similar to docker --info).

  2. @gbrey developed pyposeidon which, among other things, facilitates creating and running schism models. In pyposeidon the schism executable is provided as a path. The origin of the executable is irrelevant (compilation vs conda vs module vs whatever). As long as you provide a valid path, things should work. In this context it is important that the schism executable declares its version properly so that pyposeidon can choose the proper pre/post processing steps for the specific version.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

water-e avatar water-e commented on June 27, 2024

Hi all,
Let's bring these use cases back to the behavior:

  1. Joseph, I want the semantic version number from Git to work fine even if it doesn't solve everyones issues. Doing the semantic number on the basis of Git mechanics and automatic generation is quite common and using the executable name is not a very modern or "fail safe" way. Most software (cmake --version, python --version ... use this system) use this system.

  2. The buggy behavior was supposed to have been corrected October of last year. No one on the thread has directly said it doesn't work although I agree if you compiled it with the .git folder discarded it would not work without adding a manual override. The version reported by pschism -v in version 5.10.x should be useful and it works for me. To work, Git must be available at compile time, but not at run time.

  3. Having Git be available is not intuitively precluded under most conda-related workflows. For instance, the conda libraries we build in our group are sometimes done on a CI tool like Jenkins based on Git tags and in that sequence we do have access to Git information at compile time. @brey, the things you wrote about the executables being delivered in different directories seems unrelated to whether you had Git on your system when you did the compile. I think it should work fine.

  4. The comments above are restricted to 5.10. There is nothing we can do about v5.8.0, which is a tagged version written in stone. If folks are having problems mostly with old versions they should flag this as a problem with old versions. We can do some of the same things for v5.8 that we did for 5.10 and create 5.8.1, but repairing this years later is less compelling and there are some complications.

  5. Finally there is the question of OLDIO vs NEWIO. The reason this came up was that we were hearing "what they really want is to be able to tell OLDIO from NEWIO". Both OLDIO and NEWIO are available in v5.10 and this would be the recommendation. The truth is that the version number tells you very little about what is in your version of schism, which is why when we created the build system we stacked everything in the executable name.

@brey can you fill in about how the code is compiled for conda? Where this happens and why you wouldn't have access to git?

from schism.

pmav99 avatar pmav99 commented on June 27, 2024

@water-e The conda recipe was downloading the tar.gz files, therefore it didn't have access to the .git directory. That's why the -v didn't work. This is the relevant code

schism/conda.recipe/meta.yaml

Lines 12 to 15 in b3ba0c4

source:
# path: ..
url: https://github.com/schism-dev/schism/archive/refs/tags/v5.9.0.tar.gz
sha256: 3f990b8f005079a07a62f08f491ba2818000fb2905772ce50e2253a1334d8420

We think it should be possible to use the git repo instead, @brey is looking into it.

we stacked everything in the executable name

Just as an alternative, schism -v could also print the variables + values of the SCHISM components as defined in e.g. cmake/SCHISM.local.build. This would be both less fragile than the filename and offer more information to the users.

from schism.

josephzhang8 avatar josephzhang8 commented on June 27, 2024

from schism.

brey avatar brey commented on June 27, 2024

Dear All,

I have successfully modified the conda recipe to use git and it works as expected. After installation with conda one gets

❯ schism -v                      
 
 schism v5.10.1
 git hash 1955928f

So in terms of #13 this is not an issue anymore. This also takes care of #50 as well.

One thing, I would like to clarify is the following:

Going forward, there will be no tagging on the master branch and all tagging will be on the corresponding branch. T/F?

In terms of adding more info on the cmake flags of the build we can explore some options and come back with ideas in a new issue.

from schism.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.