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

wang_landau_reaction_ensemble.py: use np.transpose #3312

Merged
merged 1 commit into from
Nov 20, 2019

Conversation

junghans
Copy link
Member

@junghans junghans commented Nov 14, 2019

Fix #3305

@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #3312 into 4.1 will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             4.1   #3312   +/-   ##
=====================================
- Coverage     85%     85%   -1%     
=====================================
  Files        531     531           
  Lines      25985   25985           
=====================================
- Hits       22139   22137    -2     
- Misses      3846    3848    +2
Impacted Files Coverage Δ
src/core/particle_data.cpp 96% <0%> (-1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b7157a...491d3d3. Read the comment docs.

@mkuron
Copy link
Member

mkuron commented Nov 14, 2019

Needs to be cherry picked to the master branch.

@jngrad
Copy link
Member

jngrad commented Nov 15, 2019

@junghans I've started a discussion in our group to find a more efficient release workflow. Could it be feasible on your side to run the Fedora 32 builds on a PR instead of a tagged release? This would help us integrate your fixes a few days before the official release. Until now we have pushed your fixes to the release branches outside our release schedule, which is confusing for the end user.

@junghans
Copy link
Member Author

How about triggering CoPR builds as part of the CI, https://pavel.raiskup.cz/blog/copr-ci-and-custom-source-method.html.

I did something like that here: https://github.com/junghans/espresso-rpm

@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Nov 15, 2019 via email

@fweik
Copy link
Contributor

fweik commented Nov 15, 2019

So what else should we do to avoid finding bugs immediately after every release?

@junghans
Copy link
Member Author

For VOTCA we usually tag an rc and then wait a week and make the full release if no bugs were found.

@jngrad jngrad mentioned this pull request Nov 15, 2019
@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Nov 15, 2019 via email

@mkuron
Copy link
Member

mkuron commented Nov 15, 2019

We could try to automate the release process to make bugfix releases less painful. Then we could simply release more often, e.g. a 4.1.2 that comes just a few days after the 4.1.1.

@jngrad
Copy link
Member

jngrad commented Nov 15, 2019

The issue with RC is that only @junghans will use it. It takes time to make a release due to the slow QEMU tests in CI and code reviews. Is this effort really worth it, considering the same amount of work can be used to produce instead two bugfix releases? That's why I'd like to know how much trouble it is to use a PR instead of a tagged release for the RPM.

@fweik
Copy link
Contributor

fweik commented Nov 15, 2019

@RudolfWeeber is was more thinking of #3305 than of #3310, which could have been identified by running the package build before the release.

@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Nov 15, 2019 via email

@junghans
Copy link
Member Author

@jngrad it is possible to use any git sha for the release, but it is preferred to package a released version. There is also nothing wrong with patch stuff into the RPM as I did for previous releases.

With the CoPR + webhook setup similar to https://github.com/junghans/espresso-rpm, you could test a release from a sha on at least half of the archs using CoPR. For OpenSuse you could do the same by creating a branches package like e.g. https://build.opensuse.org/package/show/home:cjunghans:branches:devel:languages:python:numeric/python3-espressomd

bors bot added a commit that referenced this pull request Nov 15, 2019
3314: Modernize core r=fweik a=jngrad

Follow-up to #3304

Description of changes:
- cherry-pick #3312
- add a `BoxGeometry::volume` method
- use `bool` for true/false flags
- reduce scope of local variables
- remove obsolete comments

Co-authored-by: Christoph Junghans <junghans@lanl.gov>
Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
@jngrad
Copy link
Member

jngrad commented Nov 18, 2019

Alternative: we could decouple bugfixes from compatibility fixes. That is, once bugfix supports ends on a minor release, we keep supporting compatibility on more recent operating systems for a little longer, outside the release schedule. This would typically involve updating CMake files + bash scripts and deprecated numpy functions in testsuite files; no change in the core that could affect simulation results. We also communicate to the community that these compatibility fixes are meant to help with reproducibility and porting of old simulation scripts. The difference between 4.1 and 4.1.1 then becomes clear. This approach doesn't require any commitment from our side: we only provide a compatibility fix when one is readily available, i.e. submitted by the community, or backported from the latest release, and only for python releases, i.e. 4.0.2 and 4.1.X. We will still limit tech support on the mailing list and GitHub to the latest release.

@jngrad jngrad added this to the 4.1.2 milestone Nov 18, 2019
@jngrad jngrad merged commit a1938b8 into espressomd:4.1 Nov 20, 2019
@jngrad
Copy link
Member

jngrad commented Nov 20, 2019

@junghans @RudolfWeeber I'm currently testing Copr: jngrad/espresso-ci, Build 1113559

@junghans junghans deleted the wl_fix branch November 20, 2019 16:07
@junghans
Copy link
Member Author

@junghans @RudolfWeeber I'm currently testing Copr: jngrad/espresso-ci, Build 1113559

I would start with https://src.fedoraproject.org/rpms/espresso/blob/master/f/espresso.spec, so that you get the same flags as Fedora is using.

@jngrad
Copy link
Member

jngrad commented Nov 20, 2019

Thanks! At the moment I'm using a custom build script. Is there a way to use the instructions from the .spec file directly?

@junghans
Copy link
Member Author

My custom build script is:

#! /bin/sh -ex

git clone https://github.com/junghans/espresso-rpm.git --depth 1
cd espresso-rpm
spectool -g espresso.spec

And then Copr picks it up from there and build the rpm.

But this isn't even needed anymore these days for https://github.com/espressopp/fedora-copr, I use SCM build type rpkg and point the Clone url to the github repo.

@jngrad
Copy link
Member

jngrad commented Nov 21, 2019

After a lot of trial and error, here's my script:

#! /bin/sh -ex
git clone https://github.com/junghans/espresso-rpm.git
cd espresso-rpm
git checkout 42dde71732b7b4caea8d38a06ee24d7fbeb8551f
sed -i "s/^Version: .*/Version: 4.1.1/; s/^%setup -q$/%setup -q -n espresso/" espresso.spec
cp espresso.spec ..
spectool -g espresso.spec

Build 1114150 almost worked, it halted when some unit tests failed to find shared objects (builder-live.log.gz). Could it be a runner failure?

      Start 22: grid_test
22/68 Test #22: grid_test ........................***Failed    0.00 sec
/builddir/build/BUILD/espresso/openmpi_build/src/core/unit_tests/grid_test: error
 while loading shared libraries: EspressoCore.so: cannot open shared object file:
 No such file or directory

I can't figure out how to start a build on a specific commit, it looks like it has to download a tar.gz file. We cannot use the GitHub API to get a tar.gz from a specific commit because we rely on submodules. I tried using git clone --recursive in the setup phase, but the build starts in a different directory.

@fweik
Copy link
Contributor

fweik commented Nov 21, 2019

I think you can remove the (single) submodule and just put the source of the h5xx package into libs (as we do with e.g. Random123). That is a very low activity repo. This would probably also mean we could use automatic release tarballs?

@jngrad
Copy link
Member

jngrad commented Nov 21, 2019

There's no guarantee there won't be a new submodule in the future, e.g. Matheval.
As a workaround, I could git clone submodules in the %prep stage, if there's no other way.

@junghans
Copy link
Member Author

The spec file already has support for git: https://github.com/junghans/espresso-rpm/blob/copr/espresso.spec#L1-L3, you just need to change that.

@jngrad
Copy link
Member

jngrad commented Nov 21, 2019

If I understand the script correctly, when the variable %{git} is set, the first occurrence of Source0 is used, which generates this link using the GitHub API. This archive doesn't contain a .git folder and libs/h5xx is empty. How does Copr figure out where on GitHub to find h5xx and which commit version to fetch?

@RudolfWeeber
Copy link
Contributor

RudolfWeeber commented Nov 21, 2019 via email

@junghans
Copy link
Member Author

If I understand the script correctly, when the variable %{git} is set, the first occurrence of Source0 is used, which generates this link using the GitHub API. This archive doesn't contain a .git folder and libs/h5xx is empty. How does Copr figure out where on GitHub to find h5xx and which commit version to fetch?

You are correct about %{git}. And I always built without h5xx before. You could add a Source1: line and point it to the h5xx source and then do something like %setup -T -a 1.

@jngrad
Copy link
Member

jngrad commented Nov 22, 2019

It finally worked: Build 1115985

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.

5 participants