Skip to content

Fixing the Decay String for GeomDecay#41

Merged
nkapila6 merged 2 commits intoknakamura13:mainfrom
keerthivishal:main
May 13, 2025
Merged

Fixing the Decay String for GeomDecay#41
nkapila6 merged 2 commits intoknakamura13:mainfrom
keerthivishal:main

Conversation

@keerthivishal
Copy link

GeomDecay String method is giving only the initTemp and not the entire string thus not matching ArithDecay and ExpDecay. Fixing the same for consistency and correct result string.

image

@nkapila6
Copy link
Collaborator

Nice find!

Can you fix the corresponding test too?

https://github.com/knakamura13/mlrose-ky/actions/runs/13536033580/job/37828163496?pr=41

@nkapila6
Copy link
Collaborator

I need to see why it was implemented this way because the repr method gives out the correct class name.

https://github.com/knakamura13/mlrose-ky/blob/main/src/mlrose_ky/algorithms/decay/geom_decay.py

@keerthivishal
Copy link
Author

keerthivishal commented Feb 26, 2025

Sure, please confirm if there is any reason why its done like that. the repr in other decay just calls the str method. I will wait for your confirmation before updating the test case.

@keerthivishal
Copy link
Author

Nice find!

Can you fix the corresponding test too?

https://github.com/knakamura13/mlrose-ky/actions/runs/13536033580/job/37828163496?pr=41

fixed the test case as well.

@nkapila6
Copy link
Collaborator

Looks good to me.

Thanks for your PR!

@nkapila6 nkapila6 merged commit 524897a into knakamura13:main May 13, 2025
1 check passed
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.

2 participants