Skip to content

Conversation

@agcopenhaver
Copy link
Collaborator

@agcopenhaver agcopenhaver commented Aug 15, 2024

Explicitly setting llmin and llmax, cleanup reclass to make it flexible if n_classes changes and more readable using a loop.

Fixes the problem noted in #31.

I'd also like to change the alternative approach accordingly as well. But wanted to keep this PR small.

@agcopenhaver agcopenhaver added the bug Something isn't working label Aug 15, 2024
@YaoTingYao
Copy link
Member

YaoTingYao commented Aug 19, 2024

Hi @agcopenhaver, thank you for identifying the precision issue. I agree that risk_class[n_classes-1][1] can experience precision problems with the default float64 datatype. Also, since risk_class[0][0] will naturally be set to NRT due to x[0][0] being 1, it might be cleaner to remove the explicit assignment risk_class[0][0] = NRT. What do you think? The proposed loop looks great to me. Cheers!

@YaoTingYao
Copy link
Member

Since the statement risk_class[0][0] = NRT would not cause any memory or performance issues and helps users understand the arrangement of risk_class, the PR is good to go.

@YaoTingYao YaoTingYao merged commit 62499ed into main Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants