Implement first draft of freight lem execution sequence #120

Konnde · 2024-11-27T09:34:32Z

First iteration implementation to run freight forecasting in lem model system environment. Draft uses similar user argument parsing to lem.py.

Currently performs demand calculation and traversal data handling for each freight parameter given in params path. User can specify eligible commodity names that are also assigned to network. Commodity extra attribute creation is only performed for specified commodities.

This PR is currently open for discussion.

…truck commodities. Changed freight spec perception value to improve result interpretation.

…extra attribute.

…Added new traversaldata handling module. Added docstring to outputting traversal matrices. Fixed commodity typo in testing freight assignment.

…t specified in run arguments. Small traversaldata refactoring.

Konnde · 2024-11-27T09:36:33Z

Scripts/freight_lem.py

+            continue
+        purposes[commodity] = FreightPurpose(commodity_params, zonedata, resultdata)
+    purps_to_assign = list(filter(lambda purposes: purposes[0] in
+                                  list(purposes), args.specify_commodity_names))


Create extra attributes for only specified purposes.

Konnde · 2024-11-27T09:39:26Z

Scripts/freight_lem.py

+        commodity_params = json.loads(file.read_text("utf-8"))
+        commodity = commodity_params["name"]
+        if commodity not in ("marita", "kalevi"):
+            continue


Forgot to take this off while testing. Shouldn't be here.

So will you remove it?

Konnde · 2024-11-27T09:41:21Z

Scripts/datahandling/traversaldata.py

+    aux_tons = numpy.zeros([len(zones), len(zones)], dtype=numpy.float32)
+    for ass_class in param.freight_modes:
+        file = result_path / f"{ass_class}.txt"
+        aux_tons += read_traversal_file(file, numpy.array(zones))


I was also thinking of passing the zeros matrix to the next func and performing addition there but this seems better to me.

Konnde · 2024-11-27T09:43:15Z

Scripts/assignment/emme_assignment.py

@@ -183,6 +183,11 @@ def prepare_freight_network(self, car_dist_unit_cost: Dict[str, float],
                    "LINK", '@a_' + attr_name,
                    "commodity flow", overwrite=True,
                    scenario=self.mod_scenario)
+                attr_name = (comm_class + "truck")[:17]
+                self.emme_project.create_extra_attribute(
+                        "LINK", '@' + attr_name,


Commodity specific truck assignment was missing. Added into this development phase.

Konnde · 2024-11-27T09:46:02Z

Scripts/assignment/datatypes/freight_specification.py

@@ -69,7 +69,7 @@ def __init__(self,
                "perception_factor": 1,
            },
            "aux_transit_time": {
-                "perception_factor": 10,
+                "perception_factor": 30,


This is related to having difficulties interpreting aux time matrix results in current iteration due to timau * 3 in mode u. Instead I tested demand calc with having timau * 1 and perception factor 30 here which seemed to produce same results with easier aux time interpretation. We can drop this from this PR if needed.

zptro

Looks very good! Some remarks:

zptro · 2024-11-27T10:18:16Z

Scripts/datahandling/traversaldata.py

+                int(line.split(" ")[0])
+            except ValueError:
+                continue
+            filt_data = list(filter(None, line.split(" ")))


How is this different from simply line.split()?

Seems that I was stuck on splitting with white spaces and forgot that split without sep specification gets rid of them. Will update.

zptro · 2024-11-27T10:28:11Z

Scripts/datahandling/traversaldata.py

+        if char not in units.keys():
+            continue
+        value = f"{cell[:i]}.{cell[i+1:]}"
+        return numpy.float32(value) * units[char]


Consider writing without the negation:

Suggested change

if char not in units.keys():

continue

value = f"{cell[:i]}.{cell[i+1:]}"

return numpy.float32(value) * units[char]

if char in units.keys():

value = f"{cell[:i]}.{cell[i+1:]}"

return numpy.float32(value) * units[char]

zptro · 2024-11-27T10:30:52Z

Scripts/datahandling/traversaldata.py

Very nice module!

zptro · 2024-11-27T10:33:28Z

Scripts/freight_lem.py

+    base_zonedata_path = Path(BASE_FOLDER, args.baseline_data_path, BASE_ZONEDATA_FILE)
+    cost_data_path = Path(BASE_FOLDER, args.cost_data_path)
+    results_path = Path(BASE_FOLDER, args.results_path)
+    emme_project_path = Path(BASE_FOLDER, args.emme_path)


This script is meant for running in production, so it should not use paths relative to BASE_FOLDER. Would we not prefer having the paths as-is from argument parsing?

This makes sense, will update.

zptro · 2024-11-27T10:39:14Z

Scripts/freight_lem.py

+            if purpose not in args.specify_commodity_names:
+                continue
+            # Explicitly save commodity
+            matrix_id = f"mf{matrix_counter}"
+            matrix_name = f"{purpose}_{mode}"
+            matrix_type = "demand"
+            ass_model._create_matrix(matrix_id, matrix_name, matrix_type, overwrite=True)
+            matrix_counter += 1
+            ass_model.freight_network.emme_matrices.update(
+                {matrix_name: {matrix_type: matrix_id}})
+            ass_model.freight_network.set_matrix(matrix_name, demand[mode])
+            with resultmatrices.open("freight_demand", "vrk", zone_numbers, m="a") as mtx:
+                mtx[matrix_name] = demand[mode]
+        if purpose in args.specify_commodity_names:
+            ass_model.freight_network.save_network_volumes(purpose)
+            matrix_counter += 7 # Shift to next 10 digit


Let's leave this as it is for now (or remove it if the script works without it), but I will integrate it in AssignmentModel as soon I have finished the refactoring of ditto.

zptro · 2024-11-27T10:43:05Z

Scripts/freight_lem.py

+    config = {
+        "LOG_LEVEL": "INFO",
+        "LOG_FORMAT": "TEXT",
+        "SCENARIO_NAME": "test",
+        "BASELINE_DATA_PATH": "tests/test_data/Scenario_input_data/2030_test/",
+        "COST_DATA_PATH": "tests/test_data/Scenario_input_data/2030_test/costdata.json",
+        "RESULTS_PATH": "tests/test_data/Results/test/",
+        "EMME_PATH": "tests/test_data/Results/test_assignment/test_assignment.emp",
+        "FIRST_SCENARIO_ID": 19,
+        "SAVE_EMME_MATRICES": True,
+        "FIRST_MATRIX_ID": 200,
+        "SPECIFY_COMMODITY_NAMES": ["marita"]
+    }


Please remove from here. We can use exisiting Config class (taking the defaults from dev-config.json) if we want to have default values.

zptro · 2024-11-27T11:06:00Z

Scripts/freight_lem.py

+    impedance = {
+        "truck": {
+            "dist": impedance["dist"]["truck"],
+            "time": impedance["time"]["truck"],
+            "toll": zeros
+        },
+        "freight_train": {
+            "dist": impedance["dist"]["freight_train"],
+            "time": impedance["time"]["freight_train"],
+        },
+        "ship": {
+            "dist": impedance["dist"]["ship"],
+            "channel": zeros
+        },
+        "freight_train_aux": {
+            "dist": impedance["aux_dist"]["freight_train"],
+            "time": impedance["aux_time"]["freight_train"],
+            "toll": zeros
+        },
+        "ship_aux": {
+            "dist": impedance["aux_dist"]["ship"],
+            "time": impedance["aux_time"]["ship"],
+            "toll": zeros
+        }


I suggest integrating impedance_aux into impedance in calc_rail_cost, calc_ship_cost and get_aux_cost, so that names of "aux_dist" and "aux_time" would not change. The name changes "aux_dist" -> "dist" and "aux_time" -> "time" could happen inside get_aux_cost. In that way you would not have to do this hard-coded remapping here. This would then be enough here:

impedance = {mode: {mtx_type: impedance[mtx_type][mode] for mtx_type in impedance if mode in impedance[mtx_type]} for mode in ("truck", "freight_train", "ship")}

Yeah I also was not thoroughly content with this section and forgot to comment on it. The reasoning for this was that we had previously refactored freight cost calc impedance formatting into a cleaner look, which however, now contradicted assign() return formatting of freight_assignment.py. I was thinking of similar dict comprehension per suggestion but in the end opted for this longer but easier to interpret structure.

Anyway, I agree with the aux name formatting which I will update along with the dict comprehension suggestion. Besides these to points, we can let this be as it is for time being.

…tead of using an explicit aux impedance parameter in freight cost calc, refactored to store all matrices in single impedance parameter.

zptro · 2024-12-04T12:48:57Z

Scripts/datahandling/traversaldata.py

+            line = line.strip()
+            try:
+                int(line.split(" ")[0])
+            except ValueError:
+                continue
+            data = list(line.split())


Suggested change

line = line.strip()

try:

int(line.split(" ")[0])

except ValueError:

continue

data = list(line.split())

data = line.split()

try:

int(data[0])

except ValueError:

continue

zptro · 2024-12-04T12:53:46Z

Scripts/freight_lem.py

+        commodity_params = json.loads(file.read_text("utf-8"))
+        commodity = commodity_params["name"]
+        if commodity not in ("marita", "kalevi"):
+            continue


So will you remove it?

zptro · 2024-12-04T13:02:41Z

Scripts/freight_lem.py

+    parser = ArgumentParser(epilog="Freight lem-model-system entry point script.")
+    config = utils.config.read_from_file()
+    config["SAVE_EMME_MATRICES"] = True
+    config["SPECIFY_COMMODITY_NAMES"] = ["marita"]


If you really want to have this as the default, could you move it to dev-config.json?

zptro · 2024-12-04T13:17:01Z

Scripts/freight_lem.py

+    if not isinstance(args.specify_commodity_names, list):
+        raise TypeError("Invalid type for save commodity names")
+    for name in args.specify_commodity_names:
+        if name not in list(commodity_conversion):
+            raise ValueError("Invalid given commodity name.")


I think that what you want to use is the nargs="*" and choices=commodity_conversion arguments in add_argument for "--specify-commodity-names". Setting type is unnecessary when dealing with strings. See docs here: https://docs.python.org/3/library/argparse.html#nargs

zptro · 2024-12-04T13:46:50Z

Scripts/utils/freight_costs.py

+    aux_dist = impedance["aux_dist"]
+    impedance["dist"] = impedance[f"aux_dist"]
+    del impedance[f"aux_dist"]
+    try:
+        impedance["time"] = impedance[f"aux_time"]
+        del impedance[f"aux_time"]
+    except KeyError:
+        pass
+    try:
+        del impedance["channel"]
+    except KeyError:
+        pass


Modifying an existing dict can have unpredictable effects outside the function. I suggest creating a new dict:

Suggested change

aux_dist = impedance["aux_dist"]

impedance["dist"] = impedance[f"aux_dist"]

del impedance[f"aux_dist"]

try:

impedance["time"] = impedance[f"aux_time"]

del impedance[f"aux_time"]

except KeyError:

pass

try:

del impedance["channel"]

except KeyError:

pass

impedance_aux = {

"dist": impedance["aux_dist"],

"time": impedance["aux_time"],

}

Why would "aux_time" not exist?

Konnde · 2024-12-05T12:10:44Z

Good comments, addressed them to improve coherency.

A matter which I noticed is that most of the current freight json parameters currently have wrong header naming. This causes the program to pick up a purpose "freight" which obviously does not exist. I have already fixed this model naming issue on the module that creates these json files. My suggestion is that, as we know that the freight parameter files in the repo are outdated due to the recently found size issue, we'll not include the updated models into this PR. Instead I will add open a new PR where we go over these updated freight models.

zptro

Looks very good!

Konnde added 5 commits November 20, 2024 13:10

Implemented first freight lem version. Added extra attr creation for …

7368527

…truck commodities. Changed freight spec perception value to improve result interpretation.

Merge branch 'main' into feat/add_freight_lem

06ed86e

Assign and save user given commodity. Added commodity specific truck …

9bb7d21

…extra attribute.

Implemented aux demand processing through use of traversal matrices. …

fbb5bbc

…Added new traversaldata handling module. Added docstring to outputting traversal matrices. Fixed commodity typo in testing freight assignment.

Filter out commodity extra attribute creation for extra attributes no…

8c5bf0b

…t specified in run arguments. Small traversaldata refactoring.

Konnde commented Nov 27, 2024

View reviewed changes

Konnde requested a review from zptro November 27, 2024 09:47

zptro requested changes Nov 27, 2024

View reviewed changes

Konnde and others added 3 commits November 29, 2024 09:44

Fix test network freight gate extra attribute numbering. (#122)

241edd9

Performed refactoring on freight lem structure and traversaldata. Ins…

a9cee06

…tead of using an explicit aux impedance parameter in freight cost calc, refactored to store all matrices in single impedance parameter.

Updated freight demand test impedance formatting.

93eace2

Konnde requested a review from zptro December 4, 2024 11:31

zptro requested changes Dec 4, 2024

View reviewed changes

zptro changed the base branch from main to dev December 4, 2024 14:05

Performed small refactoring based on given comments.

58e123c

Konnde marked this pull request as ready for review December 5, 2024 12:47

Konnde requested a review from zptro December 5, 2024 12:48

zptro approved these changes Dec 5, 2024

View reviewed changes

Konnde merged commit 271e851 into dev Dec 5, 2024
2 checks passed

Konnde deleted the feat/add_freight_lem branch December 5, 2024 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement first draft of freight lem execution sequence #120

Implement first draft of freight lem execution sequence #120

Konnde commented Nov 27, 2024 •

edited

Loading

Konnde Nov 27, 2024

Konnde Nov 27, 2024

zptro Dec 4, 2024

Konnde Nov 27, 2024

Konnde Nov 27, 2024

Konnde Nov 27, 2024 •

edited

Loading

zptro left a comment

zptro Nov 27, 2024

Konnde Nov 27, 2024

zptro Nov 27, 2024

zptro Nov 27, 2024

zptro Nov 27, 2024

Konnde Nov 27, 2024

zptro Nov 27, 2024

zptro Nov 27, 2024

zptro Nov 27, 2024

Konnde Nov 27, 2024

zptro Dec 4, 2024

zptro Dec 4, 2024

zptro Dec 4, 2024

zptro Dec 4, 2024

zptro Dec 4, 2024

Konnde commented Dec 5, 2024

zptro left a comment

Implement first draft of freight lem execution sequence #120

Implement first draft of freight lem execution sequence #120

Conversation

Konnde commented Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Konnde Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

zptro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Konnde commented Dec 5, 2024

zptro left a comment

Choose a reason for hiding this comment

Konnde commented Nov 27, 2024 •

edited

Loading

Konnde Nov 27, 2024 •

edited

Loading