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

4.0.2rc #2585

Closed
wants to merge 36 commits into from
Closed

4.0.2rc #2585

wants to merge 36 commits into from

Conversation

RudolfWeeber
Copy link
Contributor

Description of changes:

PR Checklist

  • Tests?
    • Interface
    • Core
  • Docs?

KaiSzuttor and others added 13 commits March 12, 2019 15:22
remove unneeded executable permissions from some files
Fix dp3m ifdefs; dawaanr-and-bh-gpu testcase requirements
Core+Test: Changing the time step should not affect particle velocities
coulomb and elc testcases can only be run with PARTIAL_PERIODIC enabled
Fixed soft sphere cutoff calculation
Fix typo in Mac installation guide
core: Make tabulated pair IA not crash if distance is too small
Testsuite: Better testing of vs relative with have_quaternions=true
@RudolfWeeber RudolfWeeber changed the base branch from python to release-4.0.1 March 13, 2019 13:38
@fweik
Copy link
Contributor

fweik commented Mar 14, 2019

This should be merged into the 4.0 branch, not 4.0.1-release, no?

@RudolfWeeber
Copy link
Contributor Author

RudolfWeeber commented Mar 14, 2019 via email

@codecov
Copy link

codecov bot commented Mar 14, 2019

Codecov Report

Merging #2585 into release-4.0.1 will decrease coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff              @@
##           release-4.0.1   #2585   +/-   ##
=============================================
- Coverage             69%     69%   -1%     
=============================================
  Files                470     469    -1     
  Lines              31842   31839    -3     
=============================================
- Hits               22215   22206    -9     
- Misses              9627    9633    +6
Impacted Files Coverage Δ
src/core/reaction_ensemble.cpp 81% <ø> (ø) ⬆️
src/core/virtual_sites/VirtualSites.hpp 100% <ø> (ø) ⬆️
src/core/p3m.cpp 83% <ø> (ø) ⬆️
src/core/mdlc_correction.cpp 87% <ø> (ø) ⬆️
src/core/lbboundaries/LBBoundary.hpp 100% <ø> (ø) ⬆️
src/core/communication.cpp 75% <ø> (-1%) ⬇️
src/core/comfixed_global.cpp 100% <ø> (ø) ⬆️
src/core/galilei.cpp 0% <ø> (ø) ⬆️
src/core/halo.cpp 57% <ø> (-2%) ⬇️
src/core/scafacos.cpp 76% <ø> (ø) ⬆️
... and 29 more

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 2e7b6e4...9aa8b76. Read the comment docs.

@fweik fweik changed the title 4.0.2rc WIP: 4.0.2rc Mar 15, 2019
fweik and others added 8 commits April 23, 2019 11:03
LBGPU: Don't forget boundaries on on_cell_strucutre_change()
2719: BH-gpu magnetostatics: multiple CPU support and refactoring r=fweik a=psci2195

Fixes espressomd#2500
In general, this fix is important cause it allows a user to parallelize both CPU and GPU calculations: EOM integration and magnetostatics respectively.

Description of changes:
 - Correct dipole prefactor broadcasting among the nodes
 - New test dds-and-bh-gpu.py is running over 4 CPUs: a comparison between BH and DDS gpu calculations. A new test has been needed because we cannot use dds-cpu as a baseline cause it does not support multiple CPUs.
- Some technical debt has been caught up here as well: smart pointers based BH, the AMD GPU skip cleanup, per node test seeding.

PR Checklist
------------
 - [ ] Tests?
   - [ ] Interface
   - [ ] Core
 - [ ] Docs?

Co-authored-by: Bogdan Tanygin <b.m.tanygin@gmail.com>
Co-authored-by: Brian Tanygin <b.m.tanygin@gmail.com>
@RudolfWeeber
Copy link
Contributor Author

@jngrad could you please give the diff between 4.0.1 and this branch a final check and then set the release process in motion?
You can also make a decision about still including the visualizer corrections or deferring them for the next time. They will have to be tested manually in the 4.0.2 branch.

@mkuron
Copy link
Member

mkuron commented Apr 25, 2019

I have confirmed that #2751 and #2677 are safe to cherry-pick.

@fweik
Copy link
Contributor

fweik commented Apr 26, 2019

@jngrad did you push to https://github.com/espressomd/espresso/tree/upstream/release-4.0.2 on purpose? Also if some of the ci pipelines are not actually needed please cancel them. They are running the emulated tests, an clog up the CI for hours.

@jngrad
Copy link
Member

jngrad commented Apr 26, 2019

I know, will git push upstream :upstream/release-4.0.2 release-4.0.2 fix it?

@fweik
Copy link
Contributor

fweik commented Apr 26, 2019

I think you can only push from local to remote, not remote to remote. But I guess you can just rename the branch on github?

@jngrad
Copy link
Member

jngrad commented Apr 26, 2019

The GitHub interface only enable branch deletion, not renaming. Solved it by deleting then pushing again with the right name.

@fweik
Copy link
Contributor

fweik commented Apr 26, 2019

@KaiSzuttor @RudolfWeeber so far we only had branches for the minor versions, not sure how we want to go about this. I would have put the 4.0.2 on the 4.0 branch.

@RudolfWeeber
Copy link
Contributor Author

RudolfWeeber commented Apr 26, 2019 via email

@mkuron
Copy link
Member

mkuron commented Apr 26, 2019

so far we only had branches for the minor versions, not sure how we want to go about this. I would have put the 4.0.2 on the 4.0 branch.

I agree, but for some reason @KaiSzuttor insisted on keeping the 4.0 branch at 4.0.0 and introducing a branch release-4.0.1 which pointed to the same commit as the v4.0.1 tag. Common practice is that branches should be used for minor versions and tags for patch releases. That would have enabled useful things like allowing other people to post pull request against the release branch...

@mkuron
Copy link
Member

mkuron commented Apr 26, 2019

If I checkout 4.0, that's what I'd expect to get.

Actually, you should expect to get the latest 4.0.x release plus any further bug fixes that have been merged to that branch. If you check out the 4.0.0 tag, you would get precisely that release.

@fweik
Copy link
Contributor

fweik commented Apr 26, 2019

I agree with @mkuron, also I think in the end we did put 4.0.1 into the 4.0 branch (see https://github.com/espressomd/espresso/branches, no 4.0.1 branch).

@jngrad
Copy link
Member

jngrad commented Apr 26, 2019

@fweik there is a 4.0.1 branch: https://github.com/espressomd/espresso/branches/stale

@fweik
Copy link
Contributor

fweik commented Apr 26, 2019

Ok, I still think that keeping patch level branches around is odd, after all the patch release should be compatible, and the previous version contains known bugs (hence the bugfix release), and should not be used anymore.

@RudolfWeeber
Copy link
Contributor Author

RudolfWeeber commented Apr 26, 2019 via email

@jngrad jngrad closed this Apr 26, 2019
@jngrad jngrad changed the title WIP: 4.0.2rc 4.0.2rc Apr 27, 2019
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.

6 participants