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.

Sign up for GitHub

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

Jump to bottom

an ITE() helper function and count/gcc globals #207

Closed
wants to merge 30 commits into from
Closed

an ITE() helper function and count/gcc globals #207

wants to merge 30 commits into from

Conversation

tias
Copy link
Collaborator

@tias tias commented Jan 5, 2023

part of a modeling exercise. Should be 'properly' integrated and tested, potentially

@JoD
Copy link
Collaborator

JoD commented Feb 6, 2023

Overall, seems good work. I've added a couple of comments, some of which are probably due to my limited understanding of the existing codebase. There are a couple of decompositions that perhaps can be improved, and a couple of efficiency optimizations.

@JoD
Copy link
Collaborator

JoD commented Feb 6, 2023

The main thing I'm wondering is if we have tests, e.g., for the value functions. Is there a policy on what should be added or how these are tested?

@Wout4 Wout4 marked this pull request as ready for review February 9, 2023 11:50
@tias tias mentioned this pull request Feb 15, 2023
Merged
@Wout4 Wout4 closed this Feb 23, 2023
@Wout4
Copy link
Collaborator

Wout4 commented Feb 23, 2023

closed because everything is split up in smaller pr's

@Wout4 Wout4 mentioned this pull request Feb 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoD JoD JoD left review comments

@Wout4 Wout4 Wout4 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tias @JoD @Wout4