Skip to content

tckglobal: Fix erroneous -niter parsing for values bigger that 1.4e9#2258

Merged
dchristiaens merged 4 commits intoMRtrix3:masterfrom
sertopexamgio:fix_tckglobal_niter_parsing
Jan 13, 2021
Merged

tckglobal: Fix erroneous -niter parsing for values bigger that 1.4e9#2258
dchristiaens merged 4 commits intoMRtrix3:masterfrom
sertopexamgio:fix_tckglobal_niter_parsing

Conversation

@sertopexamgio
Copy link
Contributor

@sertopexamgio sertopexamgio commented Jan 12, 2021

The tckglobal command when run with -niter 10B could only run for 1410065408 iterations instead of 10000000000 iterasions. These changes fixed the parsing issue.

tckglobal: fix parsing of niter
fa49f78 
When tckglobal command was run with -niter 10B the numbers of iterations,
that run were maximally 1410065408 instead of 10000000000.
This change fixes this parsing issue.
@Lestropie
Copy link
Member

Lestropie commented Jan 13, 2021

Hey @sertopexamgio,

Awesome to see some external pull requests coming in!

For this one, rather than expanding the code out to multiple lines, you can instead exploit the fact that get_option_value() is a templated function. This is typically inferred from the type of the default value, which in this case is a macro that the compiler will presumably interpret as an int. If you were to instead explicitly invoke get_option_value<uint64_t>(), that would be fractionally neater.

Cheers!
Rob

@sertopexamgio
Copy link
Contributor Author

sertopexamgio commented Jan 13, 2021

Hey @Lestropie,

thanks for your review, that was a neat suggestion.
I hope it looks good now.
Have a good one!

Cheers!
Serxhio

dchristiaens
dchristiaens requested changes Jan 13, 2021
View reviewed changes
Copy link
Member

@dchristiaens dchristiaens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur with @Lestropie that

uint64_t niter = get_option_value<uint64_t>("niter", DEFAULT_NITER);

would be the cleaner syntax. Nice addition and thanks for contributing!

@dchristiaens dchristiaens added this to the 3.0.3 hotfix milestone Jan 13, 2021
@dchristiaens dchristiaens merged commit 63d9482 into MRtrix3:master Jan 13, 2021
@sertopexamgio sertopexamgio deleted the fix_tckglobal_niter_parsing branch January 13, 2021 16:40
@Lestropie Lestropie modified the milestones: 3.0.4 hotfix, 3.0.3 hotfix May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dchristiaens dchristiaens dchristiaens approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

3.0.3 hotfix

Development

Successfully merging this pull request may close these issues.

3 participants

@sertopexamgio @Lestropie @dchristiaens