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

arpackmm: add --slvDrtPvtThd option. #178

Merged
merged 2 commits into from
Jan 10, 2019
Merged

Conversation

fghoussen
Copy link
Collaborator

Pull request purpose

arpackmm: enhancement

Detailed changes proposed in this pull request

add control on pivot threshold if direct solver is used.

@fghoussen
Copy link
Collaborator Author

Should not increase coverage ! :D
I will likely PR another small feature by the end of the day or tomorrow : after that 3.7 should be OK (1st "good" enough version of arpackmm - probably add more for versions > 3.7 as I will need it).

@fghoussen
Copy link
Collaborator Author

Oh... Stupid : make_unique is likely C++14 ? And compiler option is C++11.
Should I move to C++14 ? Or modify the code to compile with C++11 ?
Any suggestion to bypass this stupid thing is welcome !...

@sylvestre
Copy link
Contributor

C++14 is a bit too early :/ (I know)

@fghoussen
Copy link
Collaborator Author

OK, this is just for testing.
This patch is logical to be added now, but, not so central anyway: can wait after 3.7.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 657

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 74.783%

Totals Coverage Status
Change from base Build 654: 0.0%
Covered Lines: 5080
Relevant Lines: 6793

💛 - Coveralls

@fghoussen
Copy link
Collaborator Author

@sylvestre : turns out to be OK ! Do you take it ?

@sylvestre sylvestre merged commit 127630c into opencollab:master Jan 10, 2019
@sylvestre
Copy link
Contributor

Sorry, no, I don't want to have C++14 as mandatory dep yet.

sylvestre added a commit that referenced this pull request Jan 10, 2019
sylvestre added a commit that referenced this pull request Jan 10, 2019
@sylvestre
Copy link
Contributor

Sorry about that, please reopen

@fghoussen
Copy link
Collaborator Author

Sorry, no, I don't want to have C++14 as mandatory dep yet.

OK, got it. I'll rewrite it with C++11.

Sorry about that, please reopen

OK. For master, consider reset --hard HEAD~2 ?... As you like.
Seems I can't reopen a merged PR. I'll create a new PR for this.

@sylvestre
Copy link
Contributor

Done, thanks

@fghoussen
Copy link
Collaborator Author

Done, thanks

OK, I'll PR back again quite soon !

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.

3 participants