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

Fix bad initial value for Yamamoto method #76

Merged
merged 1 commit into from Dec 4, 2023

Conversation

ronald-jaepel
Copy link
Collaborator

Change initial value from the automatic middle of the boundary space (500 & 500) to (1, 1), as an initial value of 500 can lead to numerical errors.

Also adapted yamamoto.py to PEP8.

Copy link
Contributor

@schmoelder schmoelder left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this!

Please use separate commits for formatting and fixing issues.
In terms of style, I agree, we should probably update the entire code base with a linter such as ruff. There are, however some cases where I'd like to not use the default settings. E.g., I really dislike the way it uses linebreaks with parenthesis (see separate comment).

CADETProcess/tools/yamamoto.py Show resolved Hide resolved
Change initial value from the automatic middle of the boundary space (500 & 500) to (1, 1), as an initial value of 500 can lead to numerical errors.
@ronald-jaepel
Copy link
Collaborator Author

Alright. I'd then tend to use a separate PR for formatting changes.

I've therefore removed the formatting changes from this PR and it reduces down to one commit.

@ronald-jaepel ronald-jaepel merged commit 8c25041 into master Dec 4, 2023
6 checks passed
@schmoelder
Copy link
Contributor

Hey, I just saw that this was merged into master. While not a problem here, let's try to set dev as a merge target in future. I should have checked that myself but maybe you could also have an eye on that. Thanks! :)

@schmoelder schmoelder deleted the fix/yamamoto_bad_initial_value branch December 4, 2023 09:30
@schmoelder
Copy link
Contributor

Oh, and please use the "rebase and squash" option.

@ronald-jaepel
Copy link
Collaborator Author

ronald-jaepel commented Dec 4, 2023 via email

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.

None yet

2 participants