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

math operators: percent of, round up, round down, logarithm #1603

Merged
merged 12 commits into from
Mar 1, 2023

Conversation

adodge
Copy link
Contributor

@adodge adodge commented Feb 27, 2023

image

Round up to power of 2:

image

@adodge adodge marked this pull request as ready for review February 27, 2023 02:26
@adodge adodge changed the title percent operator for math node math operators: percent, floor, ceiling Feb 27, 2023
@RunDevelopment
Copy link
Member

All of these are unary operators. Wouldn't it make more sense to conditionally hide the b operate instead of trying to involve it somehow?

@adodge
Copy link
Contributor Author

adodge commented Feb 27, 2023

"Percent" comes from a suggestion in discord. I can see the utility of combining the division and a multiplication, since taking a percentage of something seems like the most likely thing to do once you have a percent. (Evidently, at least one person would find it useful.)

"Round up" and "Round down" might need different names, but I think rounding to the nearest n is a useful generalization of rounding to the nearest 1. Some nodes only work with sizes that are multiples of 8 or 64, for example.

@adodge adodge changed the title math operators: percent, floor, ceiling math operators: percent of, round up, round down, logarithm Feb 27, 2023
@adodge
Copy link
Contributor Author

adodge commented Feb 27, 2023

this needs a rebase... I'll take a look later today

@RunDevelopment
Copy link
Member

My issue with b in rounding up and down is that it default to 0, which kinda breaks the node. Users have to actively "fix" the node to get rounding on its own.

@adodge
Copy link
Contributor Author

adodge commented Feb 28, 2023

Ahh, yeah that makes sense.

@adodge
Copy link
Contributor Author

adodge commented Feb 28, 2023

How about something like this? A separate node for rounding:

image

@joeyballentine
Copy link
Member

I think it makes sense

backend/src/nodes/nodes/utility/round.py Outdated Show resolved Hide resolved
backend/src/nodes/nodes/utility/round.py Outdated Show resolved Hide resolved
backend/src/nodes/nodes/utility/round.py Outdated Show resolved Hide resolved
@joeyballentine joeyballentine merged commit da6bae0 into chaiNNer-org:main Mar 1, 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.

None yet

3 participants