-
Notifications
You must be signed in to change notification settings - Fork 13
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
Release 0 0 6 #151
Release 0 0 6 #151
Conversation
Begin 0.0.6 development cycle
Fix some typos and provide better documentation for determining or recalculating Python module install location.
Remove target directory to ensure expected move behavior.
* Add __hash__ and serialize() instance methods to WorkSpec. * Add deserialize() class method to WorkSpec. * Add uid() instance method to WorkSpec to generate a consistent, concise and sufficiently unique string to identify a gmxapi_workspec_0_1 dataset. * Update WorkSpec documentation. * Add a CompatibilityError exception.
For simulations run in the local context, this commit allows access to the potentials used in the context after the simulation has run and while the context is still alive. This allows the Python script to interact with the object implementing the potential with any additional interface it may have. This is a temporary form of access until other means of interacting with workflow objects are devised and implemented.
Add badges for development branch.
allow full CMake-driven build and install. Fixes #60
The devel branch of gmxapi now requires the devel branch of gromacs-gmxapi
Create only the new directory we are going to use on the current rank. Restore current directory to original state at end of session.
Update Dockerfile to include documentation with the image.
* normalize and update some quotes * Add documentation to gmx.version * Make some notes about Status class, which needs some work.
* Add and fix some missing or broken documentation. * Clean up some comments. * Build docs as part of Travis-CI builds * Reformat some code and examples for readability. * Install sphinx and rtd-theme to build docs on Travis-CI.
Relates to issue #40 Functionally, add a boolean `append_output` property to MD simulations that defaults to `true`. Implemented with an optional keyword / param that, if set `false`, gives the `mdrun -noappend` behavior.
Allow control of whether output is truncated or appended.
Addresses #103 Use caching and build matrix stages to try to optimize time. Use default compiler for MPI, upgraded gcc for tMPI, but mpi4py still builds with default compiler. No clang yet. Need to build our own MPI stuff in the future.
Convert serialized data to (utf-8 encoded) byte sequence. Adds a little bit of extra code when using the json module to make pretty formatted output, but removes ambiguity and improves implementation across Python 2 and Python 3.
Handle both Python 2 and Python 3
We are observing sporadic Travis-CI job failures that look like binary incompatibilities in compiled Python modules. The `pip` cache on Travis-CI may not handle distinctions between Pythonversions, so we will disable it.
Only test against gcc-4.8, but build gromacs with and without mpi. Also, further reduce caching of packages built by pip. There seems to be some fairly onerous caching that mixes up built packages for different python versions. We should look into how best to just specify different cache directories for each of Python 2 and 3 when everything else is equal and Travis-CI gives the job the same container cache.
* Clarify binding order of Python references in ensemble_update facility. * Fix hanging MPI calls in certain configurations. * hide dead code pending redesign work.
* Clean up caching, build stages, and cross-repo compatibility testing. * Move some complexity into supporting scripts. * Switch to "python" language CI builds for better matrix management. * Improve build step behavior. partially resolves #54
[ci skip]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
README.md should have citation information for gmxapi (bioinformatics 2018).
setup.py
Outdated
@@ -210,7 +210,7 @@ def build_extension(self, ext): | |||
if build_gromacs: | |||
# TODO! We need to distinguish dev branch builds from master branch builds or always build with the | |||
# master branch of the dependency. For one thing, as is, this line needs to be toggled for every release. | |||
gromacs_url = "https://github.com/kassonlab/gromacs-gmxapi/archive/dev_5.zip" | |||
gromacs_url = "https://github.com/kassonlab/gromacs-gmxapi/archive/devel.zip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a compelling reason to use a static zip file for gromacs_url rather than a git branch (or tag) checkout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is much faster than cloning the repo and uses standard Python modules. More recently, I've found the fastest thing is git clone -depth 1 ...
, but when we move off of readthedocs.org to a push from Travis-CI, we will not need to work so hard to shave seconds off the build time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few specific comments, otherwise LGTM.
src/gmx/context.py
Outdated
# def shared_data_maker(element): | ||
# """Make a shared data element for use by dependent nodes. | ||
# | ||
# Note: outdated. This code is in limbo and should not be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the meaning of "in limbo" here? Deprecated? In the process of being refactored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Thanks. I should have tracked that better. There is a lot of code in the various repos that is currently unused or completely dead. I'm in the process of removing the dead code where it is untested, but for unused code that is tested, I have been content to make minimal changes. For this particular section, I think the code was tested and error free, but unused and inconsistent with forward-looking design. I'll take a closer look at it before letting it propagate further.
To do
- make better use of "to do"s
- remove shared_data_maker and test code, discussion, to one or more of checkpoint for data nodes #34, more execution graph node types #36, Eigen #37, Formal AllReduce operation #45, Generic data structures for graph edges and state #69
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's not really a good place to stash ideas for posterity that I can find with GitHub. Unless maybe that's what Gist is for? I would want to stash the commented out bits of context.py and of test_mpiarraycontext.py. As it is, I tried to be more consistent with the commented out code and to include comments referring to tracked issues with commit 848e5c1. If you have other ideas, please let me know.
src/gmx/status.py
Outdated
@@ -1,7 +1,9 @@ | |||
class Status(object): | |||
"""Ultimately hold status for API operation. | |||
|
|||
This should probably inherit from gmx.core.Status. | |||
Either this should be inherited by gmx.core.Status or a reference to it or it gets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax of this comment is a little hard to parse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo:
- reword comment.
- cite appropriate tracked issue or design document.
src/gmx/workflow.py
Outdated
@@ -36,6 +17,8 @@ | |||
>>> with gmx.context.Context(md.workspec) as session: | |||
... session.run() | |||
|
|||
.. This comment line seems to be necessary to help parsing in some cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's probably a better way to express that... I couldn't get Sphinx to parse the docstring in a sane way without adding a comment line to kick the parser in a way I don't understand. I'll see if I can quickly come up with a solution and, if not, reword the necessary comment to seem like it was meant to be there. If someone with RST skilz has a better answer, please let me know.
- reword or replace bizarre comment line.
Commented out some dead code more completely, but have not removed it entirely because it captures some ideas that are still being explorered, and there doesn't seem to be a good way to attach sample source code to GitHub issues. References to the appropriate tracked issue have been added. Maybe this is something that "Gist" is good for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks. Nit: might add a note on the zip source retrieval similar to our discussion (this is planned to transition to git source retrieval at some point in the future but is currently at zip retrieval to speed CI builds).
The conversation referenced by @peterkasson relates to the use of a GitHub release archive URL (zip file) rather than git based source retrieval. The PR has been approved, so follow up to the conversation has been moved to #153 |
My primary concern is just that if we pull a zip file, then unless that is automatically created every time then we have the potential for the zip file and the source tree to get out of sync. If we're pulling a designated tag from the source tree, then it's clearly defined. |
gmxpi 0.0.6 release
Interface and feature updates
gmx.version
modulegmx.exceptions
Internal
Bug fixes
Remaining tasks