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

Check for permuted indices in NL segments #92

Closed
vitaut opened this issue Mar 8, 2016 · 14 comments
Closed

Check for permuted indices in NL segments #92

vitaut opened this issue Mar 8, 2016 · 14 comments

Comments

@vitaut
Copy link
Contributor

vitaut commented Mar 8, 2016

Ill-formed .nl files can have indices permuted. This doesn't affect the NL reader because it passes the indices to the client, but NLProblemBuilder should detect it and do something about it like reporting an error.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 8, 2016

In the C, O and F segments the order doesn't matter and duplicates will overwrite older entries which is OK.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 8, 2016

Segment that need checks: V (common expressions) and L (logical constraints).

@vitaut
Copy link
Contributor Author

vitaut commented Mar 8, 2016

Done for logical constraints (L).

@vitaut
Copy link
Contributor Author

vitaut commented Mar 14, 2016

Looks like adding all objectives at once and then updating them with expressions and types is faster (and easier) than adding them incrementally and checking if they are ordered:

Benchmark          Time(ns)    CPU(ns) Iterations
-------------------------------------------------
AddObjsAtOnce            39         39   22790390                                 
AddObjsIncVector        170        174    3537068                                 
AddObjsIncArray          85         88    7982500 

https://github.com/ampl/mp/blob/master/test/nl-benchmark.cc

@vitaut
Copy link
Contributor Author

vitaut commented Mar 15, 2016

Done for objectives.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 17, 2016

Done for constraints. Need to do the same for common expressions and functions.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 20, 2016

Done for common expressions.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 20, 2016

Also check if avoiding push_back indeed improves performance as the benchmark suggests.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 22, 2016

All permuted indices are handled correctly now.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 22, 2016

Remaining work here:

  • Check if avoiding push_back improves performance
  • Make sure that NLProblemBuilder is fully tested

@vitaut
Copy link
Contributor Author

vitaut commented Mar 25, 2016

Here are nl-benchmark results on https://github.com/ampl/mp/tree/3cd123f67fb368cc3ecf49ddbd0c461ac3b750d7 (Dec 30, 2014):

Binary:

Method Time, s
nl reader 0.24
nl reader+build 1.60
ASL 2.16

Text:

Method Time, s
nl reader 1.31
nl reader+build 2.71
ASL 3.48

Tested on machine with the following specs (titan):
CPU: Intel(R) Core(TM) i7-5820K CPU @ 3.30GHz
RAM: 7885MiB

@vitaut
Copy link
Contributor Author

vitaut commented Mar 25, 2016

Results on https://github.com/ampl/mp/tree/75791139a06eb8e021b6493fe46a863956f2bb8a (Mar 25, 2016):

Binary:

Method Time, s
nl reader 0.24
nl reader+build 1.60
ASL 2.16

Text:

Method Time, s
nl reader 1.26
nl reader+build 2.69
ASL 3.49

Text slightly improved, binary the same.

@vitaut
Copy link
Contributor Author

vitaut commented Mar 28, 2016

Results on e6afdba with fixed handling of permuted indices:

Binary:

Method Time, s
nl reader 0.23
nl reader+build 1.60
ASL 2.15

Text:

Method Time, s
nl reader 1.27
nl reader+build 2.70
ASL 3.44

@vitaut
Copy link
Contributor Author

vitaut commented Mar 28, 2016

Avoiding push_back doesn't improve performance for nonlinear problems because the time is probably dominated by other factors (expression allocation?), but at least there is no need to construct temporary arrays to permute indices.

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

No branches or pull requests

1 participant