Skip to content

[FileFormats.LP] write a proper recursive descent parser #2840

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Sep 11, 2025
Merged

[FileFormats.LP] write a proper recursive descent parser #2840

merged 10 commits into from
Sep 11, 2025

Conversation

odow
Copy link
Member

@odow odow commented Sep 10, 2025

I still need to do indicator constraints.

Closes #2836
Closes #2351

@odow odow mentioned this pull request Sep 10, 2025
Closed
odow
odow commented Sep 10, 2025
View reviewed changes
test/FileFormats/LP/models/invalid_bound.lp
c3: 2 x2 + 3 x4 <= 25
bounds
0 <= x1 <= +infinity
0 >= x2 <= 10
Copy link
Member Author

Choose a reason for hiding this comment

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

This now throws "Upper bound already set" which seems okay

odow
odow commented Sep 10, 2025
View reviewed changes
test/FileFormats/LP/models/invalid_affine_term_objective.lp
@@ -1,13 +0,0 @@
\ File: lo1.lp
maximize
obj: 3 x1 + + x2 + 5 x3 + x4
Copy link
Member Author

Choose a reason for hiding this comment

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

We can handle + + now

odow
odow commented Sep 10, 2025
View reviewed changes
test/FileFormats/LP/models/invalid_affine_term_constraint.lp
obj: 3 x1 + x2 + 5 x3 + x4
subject to
c1: 3 x1 + x2 + 2 x3 = 30
c2: 2 x1 + x2 + - 3 x3 + x4 >= 15
Copy link
Member Author

Choose a reason for hiding this comment

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

We can handle + - now

odow
odow commented Sep 10, 2025
View reviewed changes
test/FileFormats/LP/LP.jl
@@ -513,7 +510,7 @@ function test_read_model1_tricky()
@test occursin("CON4: 1 V5 + 1 V6 + 1 V7 <= 1", file)
@test occursin("CON1: 1 V1 >= 0", file)
@test occursin("CON5: [ 1 Var4 ^ 2 - 1.2 V5 * V1 ] <= 0", file)
@test occursin("R1: 1 V2 >= 2", file)
@test occursin("1 V2 >= 2", file)
Copy link
Member Author

Choose a reason for hiding this comment

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

The decision to arbitrarily give the constraint a name was ... suspect. If the user didn't provide a name. We don't need to add one.

@odow odow marked this pull request as ready for review September 11, 2025 01:45
@odow
Copy link
Member Author

odow commented Sep 11, 2025

At this point, I'm kind of over-engineering this. But LP files are finicky enough that having good error messages is a nice quality of life improvement.

[ Info: invalid_after_end.lp
Error parsing LP file. Got an unexpected token on line 7:
C: 1 x <= 2
^
No file contents are allowed after `end`.
[ Info: invalid_bound_2.lp
Error parsing LP file. Got an unexpected token on line 6:
 x1 != 10 
    ^
We expected this to be an inequality like `>=`, `<=` ,or `==`.
[ Info: invalid_constraint.lp
Error parsing LP file. Got an unexpected token on line 5:
c2:  2 x1 + x2 + c2: + 3 x3 + x4 >= 15 
                   ^
We expected this to be an inequality like `>=`, `<=` ,or `==`.
[ Info: invalid_sos_constraint.lp
Error parsing LP file. Got an unexpected token on line 11:
 csos2: S2::
            ^
SOS constraints cannot be spread across lines.
[ Info: invalid_sos_set.lp
Error parsing LP file. Got an unexpected token on line 10:
 csos2: S3:: V2:2 V4:1 V5:2.5
        ^
This must be either `S1` for SOS-I or `S2` for SOS-II.
[ Info: invalid_variable_name.lp
Error parsing LP file. Got an unexpected token on line 2:
obj: 3 1x1 + x2 + 5 x3 + x4 
       ^
Got a number, But we expected this to be a new term in the expression.

@odow odow merged commit 6c4bb8c into master Sep 11, 2025
31 checks passed
@odow odow deleted the od/lp-2 branch September 11, 2025 07:59
This was referenced Sep 11, 2025
Closed
Merged
Merged
Merged
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FileFormats.LP] Size Limit for read_from_file [FileFormats.LP] write a proper parser
1 participant
@odow