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

modeldb-meta.yaml clone or copy a local repository #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nrnhines
Copy link
Member

@nrnhines nrnhines commented Feb 24, 2024

github: "local: /path/to/repository"
github: "copy: /path/to/parentfolder"

This is currently in the form of an ugly hack, but I found the workflow it supports to be very convenient.
I think it would be straightforward to clean up the implementation.

Local repository clone

                # Using
                #  github: "local: /path/to/repository"
                # in modeldb-run.yaml implies that we 'git clone /path/to/repository'.
                # This is useful to tentatively explore the effect of
                # model changes on results with different nrn versions
                # without committing to github or before being ready to
                # make a pull request

and even more convenient is a local repository copy

               # Using
                #  github: "copy: /path/to/parentfolder"
                # in modeldb-run.yaml implies that we
                #  'cp -R /path/to/parentfolder/<id> <workingdir>'
                # A copy differs from a clone in that local changes in
                # the checkout are mirrored in the copy without having to
                # be committed. Note the copy leaves out the .git and
                # x86_64 folders.

though I haven't yet bothered to leave out the .git or x86_64 folders

One improvment might be to specify a list of ids in a separate yaml file with a format like

id:
  clone:  /path/to/parent/where/id/is/located
id:
  copy: /path/to/parent/where/id/is/located

in order to avoid temporary changes to modeldb/modeldb-run.yaml

I found the copy to be more useful than the clone so copy alone would suffice. In fact it would probably suffice merely to have an environment variable like

export MODELDB_LOCAL_REPOSITORIES=$HOME/models/modeldb

and any id folder at that location that matches an id specified in getmodels or runmodels is copied from there

  github: "local: /path/to/repository"
  github: "copy: /path/to/parentfolder"
# be committed. Note the copy leaves out the .git and
# x86_64 folders.
print("\nTo be copied by runmodel %s/%s" % (github, model_id))
return model_id, model
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nrnhines : Do we need to have both of these options? i.e. I see that copy: would cover what we typically need in our development workflow. Adding local: help in any other way?

By the way, for naming itself, IMO local: is a bit better than copy:.

Copy link
Member Author

@nrnhines nrnhines Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My experience is that copy is more useful than clone since the former copies also the changes in the working checked out version (so one does not need to first commit those changes). It could easily be called "local:". However, I think the environment variable approach above export MODELDB_LOCAL_REPOSITORIES=$HOME/models/modeldb might be better than having to modify modeldb-meta.yaml. We should discuss some of the detailed positive and negative aspects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the environment variable approach above export MODELDB_LOCAL_REPOSITORIES=$HOME/models/modeldb might be better than having to modify modeldb-meta.yaml.

That's also OK. But this will also need local commits.

Anyway, I feel like the scope + impact of this decision is limited. So I think it's perfectly fine if you choose either approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important part is to have this flexibility and document it properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there needs to be any hurry with this. I'll let it ripen for a while wait til the next round of modeldb updates to see how some of the details help with the workflow.

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.

None yet

2 participants