Modification: REPACKING

Development
1

I think I have complete the REPACK mode. (without SA)

re1
re1683×683 69.8 KB

re2
re2684×687 74.9 KB

this case is BuoyanceTest with a Sphere.

The code is updated in github (P.S. this is the first time I use GITHUB, not sure I did it property)

Also, XYZReader, and resume with floating are fixed.

and note that, --repack can not work. but --repack-only and --resume works well.
I think this bug can be blamed on the data using in repacking are not free() property.

Please contact me if you have any question.

2

Helo @JoJo,

the results look very nice indeed! Don’t worry too much about SA_BOUNDARY.

And yes, you are right, the cleanup at the end of the repacking step is most likely not good enough. A proper fix would be to switch shared pointers (the C++ shared_ptr) in all places where we’re using naked pointers at the moment. This should make the cleanup considerably easier and more robust.

I’m really looking forward to merge your contributions into the GPUSPH mainline. I’m afraid the procedure you followed on GitHub is not correct, though. Here’s a short summary of what you should do:

  1. fork GPUSPH on GitHub; you can achieve this by going to the GPUSPH page on GitHub and clicking on the “Fork” button on the upper-right corner; this will create a copy of the GPUSPH repository in your GitHub’s account namespace;

  2. add your fork of GPUSPH to the remotes to your working directory. This can be done e.g. by running:

     git remote add myfork <URL-of-your-fork>

    from the command-line in your working directory. The actual <URL-of-your-fork> is something that you can get from the GitHub page of your fork, under the Code button;

  3. commit with git the changes that introduce the fix, making at least one separate commit per fix. For example, you should do one commit for the floating body resume fix, and one commit for the repacking fix; (BTW, I have pushed a fix for the XYZReader myself; it’s slightly different from the one you proposed in the other thread, so you might want to disregard your changes for that, or double check if the fix I introduced works for you too);

  4. push the committed changes to your fork; for example, if you’re working on the next branch in your local repository, this would be something like: git push myfork next.

  5. from the GitHub page of either your fork or the GPUSPH repository, you should now see the option to propose a merge of the changes you just pushed; please target the same GPUSPH branch that you’re using (e.g. if your code is based on next, then propose to merge with GPUSPH next branch).

With this I can review the changes, and then either merge them or ask for some changes if necessary.

Do ask if you’re still having troubles setting up a proper pull request. In the mean time, I’m closing the one you opened since it doesn’t really fit the correct workflow.

(Also, if you’re not too familiar with git, I recommend reading some tutorials, since it is an extremely useful tool also to keep track of the changes that you keep for your own.)

Hope I get to review your changes soon!

3

Sorry for that, and I have updated it again.
Hope I fit the correct workflow this time…

4

Hello @JoJo,

much better, but the pull request is still problematic to review, because of two issues:

  1. you did a single commit that introduces both the floating resume and the repacking fixes;
  2. you did the commit while staying on your master branch, which means that your commit also introduces all of the changes done in next in the mean time.
  3. also, I think you committed some unrelated files, such as the stuff under ointegrator which is, I assume, the old integrator.

You should be able to solve this in the following way.

First of all, do a rebase of your next branch onto the mainline next branch; if the mainline GPUSPH repository is origin in your working directory, for example, you can achieve this by doing:

    git rebase origin/next

while on your next branch. Note that there might be some small conflict due to the changes I’ve pushed to the mainline next in the meantime, but this should be trivial to fix.

Next, you need to clean-up and split your commit. In fact, I would recommend we do the floating and repacking fixes separately (the floating object fix first, and when this is done then we proceed to the repacking fix).

I’m sorry to ask you to redo some of the work, but this is the best way to ensure that we don’t “cross the streams”. It will also be good way for both of us to get some practice with the workflow before we proceed with the more complex fix! So I would recommend you do the following, after rebasing your work:

  1. make a backup of your changes;
  2. reset the branch to the upstream situation (git reset --hard origin/next);
  3. reintroduced the changes for the floating bodies only; do not commit extra files, just the changes to the code necessary for the correct save/restore;
  4. commit those changes, and push them to a new branch in your for (e.g. git push myfork next:floating-fix);
  5. create a pull request for that branch

And then we can do the same for the repacking fix.

5

thanks for the awesome information.

6

thanks my issue has been fixed.