Skip to content

Commit

Permalink
Error handling improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
David Rijsman committed Mar 5, 2024
1 parent 80c3d32 commit 393f8a6
Show file tree
Hide file tree
Showing 31 changed files with 494 additions and 256 deletions.
6 changes: 5 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ func solver(
if err != nil {
return runSchema.Output{}, err
}
last := solutions.Last()

last, err := solutions.Last()
if err != nil {
return runSchema.Output{}, err
}

output, err := check.Format(
ctx,
Expand Down
10 changes: 8 additions & 2 deletions factory/constraint_attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ func addAttributesConstraint(
continue
}

constraint.SetStopAttributes(model.Stops()[s], *stop.CompatibilityAttributes)
err := constraint.SetStopAttributes(model.Stops()[s], *stop.CompatibilityAttributes)
if err != nil {
return nil, err
}
presentInStops = true
}

Expand All @@ -32,7 +35,10 @@ func addAttributesConstraint(
continue
}

constraint.SetVehicleTypeAttributes(model.VehicleTypes()[v], *vehicle.CompatibilityAttributes)
err = constraint.SetVehicleTypeAttributes(model.VehicleTypes()[v], *vehicle.CompatibilityAttributes)
if err != nil {
return nil, err
}
presentInVehicles = true
}

Expand Down
14 changes: 9 additions & 5 deletions factory/construction.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,14 @@ func (d defaultModelFactoryImpl) NewModel(

// NewStopCluster returns a new stop cluster for the given stops.
func NewStopCluster(
stops []schema.Stop) StopCluster {
stops []schema.Stop) (StopCluster, error) {
if len(stops) == 0 {
panic("cannot create stop cluster with no stops")
return nil, fmt.Errorf("cannot create stop cluster with no stops")
}
return stopCluster{
stops: slices.Clone(stops),
centroid: CentroidLocation(stops),
}
}, nil
}

// NewPlanUnitStopClusterGenerator returns a list of stop clusters based
Expand Down Expand Up @@ -339,9 +339,13 @@ func (s *planUnitStopClusterGeneratorImpl) Generate(

for _, solutionPlanUnit := range solution.UnPlannedPlanUnits().SolutionPlanUnits() {
stops := getInputStops(solutionPlanUnit.ModelPlanUnit())
cluster, err := NewStopCluster(stops)
if err != nil {
return nil, err
}
clusters = append(
clusters,
NewStopCluster(stops),
cluster,
)
}
return clusters, nil
Expand Down Expand Up @@ -705,7 +709,7 @@ func newSolution(
return nil, err
}

return solutions.Last(), nil
return solutions.Last()
}

// HaversineDistance returns the distance between two locations using the
Expand Down
10 changes: 8 additions & 2 deletions factory/vehicles.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,22 @@ func addVehicles(
vehicle.First().SetMeasureIndex(len(input.Stops) + len(*input.AlternateStops) + idx*2)
vehicle.Last().SetMeasureIndex(len(input.Stops) + len(*input.AlternateStops) + idx*2 + 1)

constraint.SetVehicleTypeAttributes(
err = constraint.SetVehicleTypeAttributes(
vehicleType,
[]string{alternateVehicleAttribute(idx)},
)
if err != nil {
return nil, err
}
for _, alternateID := range *inputVehicle.AlternateStops {
alternateStop, err := model.Stop(data.stopIDToIndex[alternateStopID(alternateID, inputVehicle)])
if err != nil {
return nil, err
}
constraint.SetStopAttributes(alternateStop, []string{alternateVehicleAttribute(idx)})
err = constraint.SetStopAttributes(alternateStop, []string{alternateVehicleAttribute(idx)})
if err != nil {
return nil, err
}
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,6 @@ github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lN
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/nextmv-io/sdk v1.3.1-0.20240219145622-e1dff93c1b6d h1:XWN6IvUPRUqNZ8gcFzPwoBykUpHFt+OnS5snUSbBVq4=
github.com/nextmv-io/sdk v1.3.1-0.20240219145622-e1dff93c1b6d/go.mod h1:lA3YgE1vdclPwQfPH3jFo76WXx5snk9XOWX2PCMO8cc=
github.com/nextmv-io/sdk v1.3.1-0.20240219215142-f91aa22e1377 h1:+LyaM1mq5eOHE99CSDknMyZ99I2wGapbu5Nr3vLp63g=
github.com/nextmv-io/sdk v1.3.1-0.20240219215142-f91aa22e1377/go.mod h1:lA3YgE1vdclPwQfPH3jFo76WXx5snk9XOWX2PCMO8cc=
github.com/nextmv-io/sdk v1.3.1-0.20240220144909-88ba9907052f h1:OLDqQkL/FK8CX7//qZGa6A4rTnHqUPYBCMmlSupVvOk=
github.com/nextmv-io/sdk v1.3.1-0.20240220144909-88ba9907052f/go.mod h1:lA3YgE1vdclPwQfPH3jFo76WXx5snk9XOWX2PCMO8cc=
github.com/opentracing/opentracing-go v1.2.0/go.mod h1:GxEUsuufX4nBwe+T+Wl9TAgYrxe9dPLANfrWvHYVTgc=
Expand Down
12 changes: 8 additions & 4 deletions model.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,21 +343,22 @@ func (m *modelImpl) NewVehicleType(
return vehicle, nil
}

func (m *modelImpl) addExpression(expression ModelExpression) {
func (m *modelImpl) addExpression(expression ModelExpression) error {
if existingExpression, ok := m.expressions[expression.Index()]; ok {
if existingExpression.Name() != expression.Name() {
panic(fmt.Sprintf(
return fmt.Errorf(
"expression index %d already exists with name %s,"+
" expression indices must be unique,"+
" did you forget to use NewModelExpressionIndex() on"+
" a custom expression",
expression.Index(),
existingExpression.Name(),
))
)
}
} else {
m.expressions[expression.Index()] = expression
}
return nil
}

func (m *modelImpl) setConstraintEstimationOrder() {
Expand Down Expand Up @@ -422,7 +423,10 @@ func (m *modelImpl) AddConstraint(constraint ModelConstraint) error {

if registered, ok := constraint.(RegisteredModelExpressions); ok {
for _, expression := range registered.ModelExpressions() {
m.addExpression(expression)
err := m.addExpression(expression)
if err != nil {
return err
}
}
}

Expand Down
15 changes: 9 additions & 6 deletions model_constraint_attributes.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nextroute

import (
"fmt"
"slices"

"github.com/nextmv-io/sdk/common"
Expand All @@ -24,15 +25,15 @@ type AttributesConstraint interface {
SetStopAttributes(
stop ModelStop,
stopAttributes []string,
)
) error
// SetVehicleTypeAttributes sets the attributes for the given vehicle type.
// The attributes are specified as a list of strings. The attributes are not
// interpreted in any way. They are only used to determine compatibility
// between stops and vehicle types.
SetVehicleTypeAttributes(
vehicle ModelVehicleType,
vehicleAttributes []string,
)
) error
// StopAttributes returns the attributes for the given stop. The attributes
// are specified as a list of strings.
StopAttributes(stop ModelStop) []string
Expand Down Expand Up @@ -131,21 +132,23 @@ func (l *attributesConstraintImpl) VehicleTypeAttributes(vehicle ModelVehicleTyp
func (l *attributesConstraintImpl) SetStopAttributes(
stop ModelStop,
stopAttributes []string,
) {
) error {
if stop.Model().IsLocked() {
panic("cannot set stop attributes after model is locked")
return fmt.Errorf("cannot set stop attributes after model is locked")
}
l.stopAttributes[stop.Index()] = common.Unique(stopAttributes)
return nil
}

func (l *attributesConstraintImpl) SetVehicleTypeAttributes(
vehicleType ModelVehicleType,
vehicleAttributes []string,
) {
) error {
if vehicleType.Model().IsLocked() {
panic("cannot set vehicle type attributes after model is locked")
return fmt.Errorf("cannot set vehicle type attributes after model is locked")
}
l.vehicleTypeAttributes[vehicleType.Index()] = common.Unique(vehicleAttributes)
return nil
}

func (l *attributesConstraintImpl) CheckCost() Cost {
Expand Down
80 changes: 64 additions & 16 deletions model_constraint_attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,53 @@ func TestAttributesConstraint_EstimateIsViolated(t *testing.T) {
return planUnit.NumberOfStops() == 1
})

cnstr.SetStopAttributes(singleStopPlanUnits[0].Stops()[0], []string{attribute0})
cnstr.SetStopAttributes(singleStopPlanUnits[1].Stops()[0], []string{attribute1})
cnstr.SetStopAttributes(singleStopPlanUnits[2].Stops()[0], []string{})
err = cnstr.SetStopAttributes(singleStopPlanUnits[0].Stops()[0], []string{attribute0})
if err != nil {
t.Error(err)
}
err = cnstr.SetStopAttributes(singleStopPlanUnits[1].Stops()[0], []string{attribute1})
if err != nil {
t.Error(err)
}
err = cnstr.SetStopAttributes(singleStopPlanUnits[2].Stops()[0], []string{})
if err != nil {
t.Error(err)
}

sequencePlanUnits := common.Filter(model.PlanStopsUnits(), func(planUnit nextroute.ModelPlanStopsUnit) bool {
return planUnit.NumberOfStops() > 1
})

cnstr.SetStopAttributes(sequencePlanUnits[0].Stops()[0], []string{attribute0, attribute1})
cnstr.SetStopAttributes(sequencePlanUnits[0].Stops()[1], []string{attribute1, attribute2})
err = cnstr.SetStopAttributes(sequencePlanUnits[0].Stops()[0], []string{attribute0, attribute1})
if err != nil {
t.Error(err)
}
err = cnstr.SetStopAttributes(sequencePlanUnits[0].Stops()[1], []string{attribute1, attribute2})
if err != nil {
t.Error(err)
}

cnstr.SetStopAttributes(sequencePlanUnits[1].Stops()[0], []string{attribute1})
cnstr.SetStopAttributes(sequencePlanUnits[1].Stops()[1], []string{attribute1, attribute2})
err = cnstr.SetStopAttributes(sequencePlanUnits[1].Stops()[0], []string{attribute1})
if err != nil {
t.Error(err)
}
err = cnstr.SetStopAttributes(sequencePlanUnits[1].Stops()[1], []string{attribute1, attribute2})
if err != nil {
t.Error(err)
}

cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[0], []string{attribute0, attribute1})
cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[1], []string{attribute1, attribute2})
cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[2], []string{})
err = cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[0], []string{attribute0, attribute1})
if err != nil {
t.Error(err)
}
err = cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[1], []string{attribute1, attribute2})
if err != nil {
t.Error(err)
}
err = cnstr.SetVehicleTypeAttributes(model.VehicleTypes()[2], []string{})
if err != nil {
t.Error(err)
}

solution, err := nextroute.NewSolution(model)
if err != nil {
Expand Down Expand Up @@ -399,7 +429,10 @@ func TestAttributesConstraint(t *testing.T) {
vehicleTypeAttributes := []string{"attribute-1", "attribute-2"}

for _, vt := range model.VehicleTypes() {
constraint.SetVehicleTypeAttributes(vt, vehicleTypeAttributes)
err = constraint.SetVehicleTypeAttributes(vt, vehicleTypeAttributes)
if err != nil {
t.Error(err)
}
attributes := constraint.VehicleTypeAttributes(vt)
if len(attributes) != 2 {
t.Errorf(
Expand Down Expand Up @@ -427,13 +460,22 @@ func TestAttributesConstraint(t *testing.T) {
}
}

constraint.SetVehicleTypeAttributes(model.VehicleTypes()[0], []string{})
constraint.SetStopAttributes(model.Stops()[0], []string{})
err = constraint.SetVehicleTypeAttributes(model.VehicleTypes()[0], []string{})
if err != nil {
t.Error(err)
}
err = constraint.SetStopAttributes(model.Stops()[0], []string{})
if err != nil {
t.Error(err)
}

stopAttributes := []string{"attribute-2", "attribute-3"}

for _, stop := range model.Stops() {
constraint.SetStopAttributes(stop, stopAttributes)
err = constraint.SetStopAttributes(stop, stopAttributes)
if err != nil {
t.Error(err)
}

attributes := constraint.StopAttributes(stop)

Expand Down Expand Up @@ -463,10 +505,13 @@ func TestAttributesConstraint(t *testing.T) {
}
}

constraint.SetStopAttributes(
err = constraint.SetStopAttributes(
model.Stops()[0],
[]string{"A", "B", "C", "A", "B", "C"},
)
if err != nil {
t.Error(err)
}

if len(constraint.StopAttributes(model.Stops()[0])) != 3 {
t.Errorf(
Expand All @@ -475,10 +520,13 @@ func TestAttributesConstraint(t *testing.T) {
)
}

constraint.SetVehicleTypeAttributes(
err = constraint.SetVehicleTypeAttributes(
model.VehicleTypes()[0],
[]string{"A", "B", "C", "A", "B", "C"},
)
if err != nil {
t.Error(err)
}

if len(constraint.VehicleTypeAttributes(model.VehicleTypes()[0])) != 3 {
t.Errorf(
Expand Down
8 changes: 2 additions & 6 deletions model_constraint_no_mix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,12 +570,8 @@ func TestNoMixConstraint(t *testing.T) {
),
},
)
if err != nil {
t.Fatal(err)
}
isViolated, _ = cnstr.EstimateIsViolated(move)
if !isViolated {
t.Fatal("constraint is not violated, it should not fit +A-A+B[+A][-A]-B")
if err == nil {
t.Fatal("planned stop must be after the previous planned stop, stop s3 is not")
}

move, err = nextroute.NewMoveStops(
Expand Down
Loading

0 comments on commit 393f8a6

Please sign in to comment.