Refactor cost calculations for specific classes by directly passing in class during cost

[ribsthakkar]

Class specific cost calculations were implemented in a way that was not thread safe for the parallel implementation. As a result, the link cost calculations were refactored to provide the cost of traversing a link given its flow and for a class.

[sboyles]

Looks good. As a minor suggestion, passing -1 [or a negative value] for class to linkCost to exclude fixed costs altogether is a bit awkward as is (maybe use a symbolic constant like IS_MISSING or something more informative; or maybe just document this in linkCost somewhere).

[sboyles]

Also: in convexcombination.c, take a look at line 252, if rectifyFlows is FALSE c is used uninitialized (and if it is TRUE it may be out of range)

[ribsthakkar]

Looks good. As a minor suggestion, passing -1 [or a negative value] for class to linkCost to exclude fixed costs altogether is a bit awkward as is (maybe use a symbolic constant like IS_MISSING or something more informative; or maybe just document this in linkCost somewhere).

Ok I will create a constant to make it more clear.

Also: in convexcombination.c, take a look at line 252, if rectifyFlows is FALSE c is used uninitialized (and if it is TRUE it may be out of range)

I'll take a look.

[sboyles]

I tested this with NCTCOG and something is not exactly right with how it is reading the costs for each class. (At least that's my guess -- I verified that all the other networks run correctly, but NCTCOG is the only one with different fixed costs per class). I haven't had the time to dig into it more... but I see significantly different results when I run with the old binary and with this branch. I may just start Carlin off with the old code while we are figuring this out

[ribsthakkar]

I tested this with NCTCOG and something is not exactly right with how it is reading the costs for each class. (At least that's my guess -- I verified that all the other networks run correctly, but NCTCOG is the only one with different fixed costs per class). I haven't had the time to dig into it more... but I see significantly different results when I run with the old binary and with this branch. I may just start Carlin off with the old code while we are figuring this out

Did you merge this branch into where you are doing the NCTCOG runs? Were there any conflicts?

[ribsthakkar]

I tested this with NCTCOG and something is not exactly right with how it is reading the costs for each class. (At least that's my guess -- I verified that all the other networks run correctly, but NCTCOG is the only one with different fixed costs per class). I haven't had the time to dig into it more... but I see significantly different results when I run with the old binary and with this branch. I may just start Carlin off with the old code while we are figuring this out

Also, now that I think about it, the NCTCOG issues may have to do with using a cached cost calculations somewhere stored in arc.cost rather than explicitly calling the calculateCost function

[sboyles]

Did you merge this branch into where you are doing the NCTCOG runs? Were there any conflicts?

I did merge it in and resolved a few conflicts manually but I thought it was just minor things (like commenting out some of the output). Are you able to run it on NCTCOG on your end, using the 1e-4 bushes from Teams? The first iteration the gaps should be around 1e-4... when I used this branch they were much higher initially (around 1e-2).