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

Fixed issue #5734 (FreeBoy Division by zero) #6053

Merged
merged 5 commits into from
Jul 2, 2023

Conversation

aidan-mueller
Copy link
Contributor

Added comments and used more descriptive variable names for noise
channel initialization block.

Also indented the nested for loop to improve code clarity.
The reasons for doing this can be found in this answer:
https://softwareengineering.stackexchange.com/a/362796

Added comments and used more descriptive variable names for noise
channel initialization block.

Also indented the nested for loop to improve code clarity.
The reasons for doing this can be found in this answer:
https://softwareengineering.stackexchange.com/a/362796
@LmmsBot
Copy link

LmmsBot commented Jun 13, 2021

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/a6c96e21-44d6-430f-981c-ff3c08e34903/artifacts/0/lmms-1.3.0-alpha.1.217+g476add8ab-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17749?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://output.circle-artifacts.com/output/job/81a673a2-7217-42ef-a5ed-9d5a101b71d1/artifacts/0/lmms-1.3.0-alpha.1.217+g476add8ab-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17747?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/4kpgsthd78n3dqbi/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/44120268"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/0nmxn1lg7nkx4hd4/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/44120268"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://output.circle-artifacts.com/output/job/983c2c87-1b0e-4663-8842-7e84ecb2819d/artifacts/0/lmms-1.3.0-alpha.1.217+g476add8ab-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/17748?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "32e98b7bf8338479b0530d63927a4bcd0cff645f"}

@DomClark DomClark linked an issue Jun 15, 2021 that may be closed by this pull request
@DomClark
Copy link
Member

@PhysSong noted this in the associated issue:

Also, (r, s) = (0, n) is equivalent to (r, s) = (1, n - 1), so we don't have to check for r = 0 unless s = 0.

Perhaps it would be simpler to use r = 0 and s = 0 for the initial guess outside the loops, then start the inner loop from r = 1?

@aidan-mueller
Copy link
Contributor Author

aidan-mueller commented Jul 6, 2021

@DomClark You are correct. I actually saw that comment, but for some reason I didn't quite understand what he was saying until now (facepalm).

Edit:

Perhaps it would be simpler to use r = 0 and s = 0 for the initial guess outside the loops, then start the inner loop from r = 1?

I was thinking the same thing.

I'll add a commit to change this, and perhaps while I'm at it I'll also spend a bit of time trying to optimize further if I can.

Allows us to skip r = 0 and a conditional in the loop below.
@PhysSong PhysSong merged commit 1f30ffc into LMMS:master Jul 2, 2023
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.

FreeBoy - Division by zero
5 participants