Skip to content

Octree with time-dependent size for the eccentric shearing sheet#860

Open
CsPeti05 wants to merge 60 commits into
hannorein:mainfrom
CsPeti05:nick-edits
Open

Octree with time-dependent size for the eccentric shearing sheet#860
CsPeti05 wants to merge 60 commits into
hannorein:mainfrom
CsPeti05:nick-edits

Conversation

@CsPeti05
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Owner

@hannorein hannorein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite clean! But there are several white space change (spaces, tabs, etc). Try removing them. This way, the important changes are easier to see and the history is cleaner.

Comment thread examples/shearing_sheet/problem.c Outdated
Comment thread src/boundary.c Outdated
Comment thread src/rebound.c Outdated
Comment thread src/tree.c Outdated

static struct reb_treecell *reb_tree_add_particle_to_cell(struct reb_simulation* const r, struct reb_treecell *node, int pt, struct reb_treecell *parent, int o){
struct reb_particle* const particles = r->particles;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of whitespace changes in this file.

Comment thread src/tree.c Outdated
reb_simulation_add(r, reinsertme);
}
}
int oldpos = node->pt;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whitespace change.

@hannorein
Copy link
Copy Markdown
Owner

You also need to bring the branch up to date with the the main branch since there have been changes there during the same time.

@CsPeti05
Copy link
Copy Markdown
Author

The tabs were added because VS Code used four spaces instead of one tab, and GitHub shifted the lines when I uploaded the code. Because of this, I set the indentation to be in terms of tabs. I tried to separate these changes into different commits so they do not make the actual changes less readable, but unfortunately, GitHub shows all changes at the same time here. I think if I change tabs back to spaces, the lines will be randomly shifted compared to each other, which would make it much harder to read the code. Do you have any suggestions?

@CsPeti05
Copy link
Copy Markdown
Author

You also need to bring the branch up to date with the the main branch since there have been changes there during the same time.

I am not exactly sure how to do that since I want to update the code without changing the files I edited. Do you have any tips?

@hannorein
Copy link
Copy Markdown
Owner

Regarding the whitespaces, you just need to go through line by line, undoing all the changes. The tab/spaces in REBOUND are by no means consistent, but it doesn't make sense to change them in the same pull request as adding a new feature.

Regarding the upstream merge, you need to google the precise git commands, but the basic idea is:

  1. Pull the main branch from my repo into a branch on your local repo.
  2. Attempt to merge the main branch into your branch.
  3. There will be conflicts, so you need to check those files manually and resolve them.
  4. Then commit the fixed files to your branch and push it to update the pull request.

@CsPeti05
Copy link
Copy Markdown
Author

Regarding the whitespaces, you just need to go through line by line, undoing all the changes. The tab/spaces in REBOUND are by no means consistent, but it doesn't make sense to change them in the same pull request as adding a new feature.

Regarding the upstream merge, you need to google the precise git commands, but the basic idea is:

  1. Pull the main branch from my repo into a branch on your local repo.
  2. Attempt to merge the main branch into your branch.
  3. There will be conflicts, so you need to check those files manually and resolve them.
  4. Then commit the fixed files to your branch and push it to update the pull request.

I merged the rebound/main branch into my branch after resolving the conflicts; however, many of the checks failed. I presume these checks should be successful. Did I do something wrong, maybe? The eccentric shearing sheet example runs smoothly.

@hannorein
Copy link
Copy Markdown
Owner

Yes, the unit tests should pass.

@CsPeti05
Copy link
Copy Markdown
Author

Yes, the unit tests should pass.

Do you have anything in mind that I should look at? I read the error messages and reviewed the code, but I am still stuck. Also, what is the best way to rerun the unit tests?

@hannorein
Copy link
Copy Markdown
Owner

@CsPeti05 These REBOUNDx error are not coming from your changes, so you can ignore those!

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.

3 participants