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

Use regular command line flags for applicable options #566

Merged
merged 2 commits into from
Sep 18, 2018

Conversation

rdumusc
Copy link

@rdumusc rdumusc commented Sep 17, 2018

The modified options are:
--enable-benchmark
--parallel-rendering
--synchronous-mode

@karjonas
Copy link
Contributor

There are some more options you could change to flags if you search for po::value.

@rdumusc
Copy link
Author

rdumusc commented Sep 17, 2018

The 4 in GeometryParameters could be changed if you want? But the others in RenderingParameters and StreamParameters have a negative effect because they are on by default; so I can't make them a flag without changing their names.

@karjonas
Copy link
Contributor

I don't really mind either way but it might be weird to have some bool flags take an argument and some not. We could for instance rename stream-use-compression to stream-disable-compression if that makes sense?

@rdumusc
Copy link
Author

rdumusc commented Sep 17, 2018

Yes, we could do that, I was afraid of changing too many things.
So that would be:
stream-use-compression off -> stream-disable-compression
accumulation off -> disable-accumulation | no-accumulation
head-light off -> disable-head-light | no-headlight
?

@rdumusc
Copy link
Author

rdumusc commented Sep 17, 2018

Note that the way they are now allows for the default value to change "transparently" for the user, whereas the change makes the default values explicit.

@karjonas
Copy link
Contributor

I think that makes sense.

@rdumusc rdumusc force-pushed the master branch 2 times, most recently from 2bf6437 to 150412b Compare September 17, 2018 15:11
@rdumusc
Copy link
Author

rdumusc commented Sep 17, 2018

Done. Full list of changes is:

Application parameters:
--enable-benchmark
--parallel-rendering
--synchronous-mode

Geometry parameters:
--circuit-mesh-transformation
--circuit-uses-simulation-model
--morphology-use-sdf-geometries
--morphology-dampen-branch-thickness-changerate

Rendering parameters:
--accumulation off => --disable-accumulation
--head-light off => --no-head-light

Stream parameters:
--stream-use-compression off => --stream-disable-compression

Application parameters:
--enable-benchmark
--parallel-rendering
--synchronous-mode

Geometry parameters:
--circuit-mesh-transformation
--circuit-uses-simulation-model
--morphology-use-sdf-geometries
--morphology-dampen-branch-thickness-changerate

Rendering parameters:
--accumulation off => --disable-accumulation
--head-light off => --no-head-light

Stream parameters:
--stream-use-compression off => --stream-disable-compression
@rdumusc
Copy link
Author

rdumusc commented Sep 18, 2018

retest this please

@rdumusc
Copy link
Author

rdumusc commented Sep 18, 2018

FYI I observed another failure with the throttle test, see:
https://bbpcode.epfl.ch/ci/job/oss.Brayns.github/1760/build_type=Release,platform=bb5/console

Running 7 test cases...
Entering test module "throttle"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(29): Entering test case "timeout_not_cleared"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(29): Leaving test case "timeout_not_cleared"; testing time: 272us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(38): Entering test case "timeout_with_clear"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(38): Leaving test case "timeout_with_clear"; testing time: 140us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(57): Entering test case "timeout_set_while_not_cleared"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(57): Leaving test case "timeout_set_while_not_cleared"; testing time: 250us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(68): Entering test case "timeout_clear_while_already_done"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(68): Leaving test case "timeout_clear_while_already_done"; testing time: 130us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(78): Entering test case "throttle_spam_limit"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(93): error: in "throttle_spam_limit": 9
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(78): Leaving test case "throttle_spam_limit"; testing time: 18193us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(96): Entering test case "throttle_spam_check_delayed_call"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(104): error: in "throttle_spam_check_delayed_call": check numCalls == 3 has failed [2 != 3]
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(96): Leaving test case "throttle_spam_check_delayed_call"; testing time: 10048us
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(107): Entering test case "throttle_one"
/gpfs/bbp.cscs.ch/ssd/nix-tmp/nix-build-brayns-latest.drv-1/Brayns-93c753d/tests/throttle.cpp(107): Leaving test case "throttle_one"; testing time: 146us
Leaving test module "throttle"; testing time: 29256us

*** 2 failures are detected in the test module "throttle"

@karjonas
Copy link
Contributor

Yes, I think @rolandjitsu and @tribal-tec will fix that soon.

@rdumusc
Copy link
Author

rdumusc commented Sep 18, 2018

OK cool. I'm merging this now if there are no objections from the others.

@rdumusc rdumusc merged commit bb036bc into BlueBrain:master Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants