Skip to content

[SYSTEMDS-3343] Boolean Arguments in GridSearch#1575

Closed
Baunsgaard wants to merge 2 commits into
apache:mainfrom
Baunsgaard:evalBoolean
Closed

[SYSTEMDS-3343] Boolean Arguments in GridSearch#1575
Baunsgaard wants to merge 2 commits into
apache:mainfrom
Baunsgaard:evalBoolean

Conversation

@Baunsgaard
Copy link
Copy Markdown
Contributor

Boolean arguments in lists evaluate to false behind eval calls.

@Baunsgaard
Copy link
Copy Markdown
Contributor Author

This PR contains some tests that currently incorrectly map boolean arguments to the functions through eval calls.
I hypothesize that it has to do with conversion to scalars somewhere for our boolean arguments.
The evaluation is correct if the boolean arguments are part of the grid, but not if they are set to default values.

@Baunsgaard
Copy link
Copy Markdown
Contributor Author

@mboehm7
It seems like your fix to boolean arguments ( a0987e5 ) with defaults did not fix the issue.

@Baunsgaard
Copy link
Copy Markdown
Contributor Author

Tests are in
src/test/java/org/apache/sysds/test/functions/mlcontext/MLContextTest.java

  • testExecuteEvalGridSearchNoDefault
  • testExecuteEvalGridSearchWithDefault
  • testExecuteEvalGridSearchWithTwoDefault

@mboehm7
Copy link
Copy Markdown
Contributor

mboehm7 commented May 17, 2022

I did not try out this PR here yet, but there was no issue with boolean defaults. My recent changes first introduced that we support defaults in eval calls at all. In the past, gridSearch simply fixed verbose=FALSE but I added tests which now check that gridSearch works correctly for both verbose configurations.

@Baunsgaard
Copy link
Copy Markdown
Contributor Author

My tests here, might be different since i here call methods that have no default value,
but instead my default value is set inside the lists.

@asfgit asfgit closed this in 70fca95 May 17, 2022
@Baunsgaard Baunsgaard deleted the evalBoolean branch August 18, 2022 12:06
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