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

Rewrite and overhaul of AmplNLWriter. #125

Merged
merged 22 commits into from
Apr 11, 2021
Merged

Rewrite and overhaul of AmplNLWriter. #125

merged 22 commits into from
Apr 11, 2021

Conversation

odow
Copy link
Member

@odow odow commented Apr 8, 2021

Ideally, some, or all of this would be migrated into
MOI.FileFormats as an NL submodule.

This needs many more tests before merging.

This implementation is simplier than the current one, because it
doesn't try to detect or simplify linear expressions.

Closes #15 (They're commented out for a reason, and now much better explained)
Closes #72
Closes #108 (We don't need persistent storage between sessions.)
Closes #123

odow added 2 commits April 8, 2021 21:25
Ideally, some, or all of this would be migrated into
MOI.FileFormats as an NL submodule.

This needs many more tests before merging.

This implementation is simplier than the current one, because it
doesn't try to detect or simplify linear expressions.
@odow odow changed the title WIP: begin a rewrite and overhaul of AmplNLWriter. Rewrite and overhaul of AmplNLWriter. Apr 9, 2021
@odow
Copy link
Member Author

odow commented Apr 9, 2021

Okay, this is now passing all tests so I'm pretty confident there aren't any lingering bugs.

I still need to benchmark against the old version, but I'd expect this version to be significantly quicker. I have some ideas for other improvements, but that can wait for a different PR.

On the whole, this version is much more maintainable. A lot of the core code was untouched since 2016, and I've added a lot of comments throughout on the format. Unfortunately the PDFs aren't the most descriptive for some features.

src/AmplNLWriter.jl Outdated Show resolved Hide resolved
@odow
Copy link
Member Author

odow commented Apr 11, 2021

Okay, the design is a lot better now. I don't know why I didn't consider it before. This PR is large and unwieldy, so I'm going to merge it as is, and make some smaller follow-up PRs. I'll hold off tagging a new version for a while to give people time to try out the #master version.

@odow odow merged commit 97d5e22 into master Apr 11, 2021
@odow odow deleted the od/rewrite branch April 11, 2021 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Julia Scratch Spaces clean_solverdata and tmp files Commented out functions in nl_params.jl
2 participants