cctbx / cctbx_project

Computational Crystallography Toolbox
https://cci.lbl.gov/docs/cctbx
Other
207 stars 111 forks source link

Reduce2 speedup #964

Closed russell-taylor closed 4 months ago

russell-taylor commented 4 months ago

@bkpoon It looks like when I ran libtbx.clean_clutter, it caused conflicts that must be resolved before the merge. None of these are in files that I modified. What is the best way to proceed?

russell-taylor commented 4 months ago

@bkpoon It looks like they will be straightforward to resolve using the Github tools. If the pull requests passes the checks, I'll go ahead and give that a try.

russell-taylor commented 4 months ago

@bkpoon Actually, there is a non-trivial difference in mmtbx/ligands/hierarchy_utils.py where I don't know whether the removal of the code is the right thing to do or not. Can you have a look at that and let me know?

bkpoon commented 4 months ago

There were some recent changes to mmCIF/PDB handling. Did you make many commits? It may be easier to create a new branch starting with master and then cherry-picking your new commits.

russell-taylor commented 4 months ago

I made enough that I'm not confident that I can pick them out of the 377 total commits. If you can easily determine the right thing to do with the one I pointed out, I'm okay doing the merge. Otherwise, I'll probably try checking the new files and old files out separately and just jamming the new ones in with a commit that has all of the comments I can find.

I tried a rebase, but it failed before I did the merge with master.

bkpoon commented 4 months ago

Let me see if I can cherry-pick into a new branch. In the list shown above, it should the commits that only have you as the committer.

nksauter commented 4 months ago

Just a heads up. I plan to merge PR#957 at 12:00 Pacific today.

bkpoon commented 4 months ago

That should be fine. I want to get a branch with just Russell's changes that can be rebased on top of master.

bkpoon commented 4 months ago

This should show @russell-taylor commits since Nov 1, 2023 in the reduce2-speedup branch. https://github.com/cctbx/cctbx_project/commits/reduce2-speedup?author=russell-taylor&since=2023-11-01&until=2024-02-06

bkpoon commented 4 months ago

I made a new branch named r2s I got the hashes with

git log --author="Russ Taylor" --after="Nov 1 2023" --pretty=format:"%h" --reverse > hash.txt

Then I cherry-picked each hash into a new branch with the latest changes with

for hash in `cat hash.txt`; do git cherry-pick $hash; done

@russell-taylor Can you check the r2s branch to see if I missed anything? You can reproduce the steps in a new branch so that GitHub will have the same person committing and pushing. I did remove the clutter commit since it was not longer needed.

russell-taylor commented 4 months ago

I get conflicts when I try to cherry-pick those, in xfel/euxfel/definitions for one of them. It looks like it was a commit that you and I jointly made?

russell-taylor commented 4 months ago

Sorry, I'll check the rs2 branch.

russell-taylor commented 4 months ago

Closing in favor of Billy's better branch.