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

Automatically drop unneeded columns in choosers table #833

Merged
merged 19 commits into from
Apr 21, 2024

Conversation

i-am-sijia
Copy link
Contributor

@i-am-sijia i-am-sijia commented Mar 18, 2024

This PR addresses #792

  • Drop unused columns automatically based on UECs
  • Pick up variables used/defined in source code, e.g. custom chooser
  • Skip dropping when running household debug tracing
  • Skip dropping when running estimation mode
  • Pass all existing CI tests
  • Run benchmarking runs
  • Clean up codes, consolidate dup codes into util funtions

@i-am-sijia
Copy link
Contributor Author

Note that when dropping columns in pandas df, the memory actually goes up momentarily before it goes down. see discussion here pandas-dev/pandas#17092, this issue was "closed" but it did not get resolved.

Looking at my code changes last week, I realized I was also dropping the columns a bit too late for interaction simulate models. The columns should be dropped before interaction df is created. I then made those changes.

Below shows the 1-zone benchmarking runs with main branch (top) and with this PR (bottom). In the non-sharrow mode, the peak memory went down from 352 GB to 261 GB, a 25% reduction.

run time comp

Component-wise memory reduction, sorted by memory saving:

image

I'm still dealing with some crashes in the sharrow mode.

@jpn--
Copy link
Member

jpn-- commented Mar 26, 2024

this issue was "closed" but it did not get resolved

If you read the thread closely, you'll note that that "this is not going to be solved in pandas 1." The behind-the-curtain memory management of pandas 1.x doesn't offer any way to drop unused columns without consuming extra RAM. This is alleviated by moving to pandas 2... after the transition, there are now ways to get the memory benefit. This is (part of) the reason I've worked this past week to try to get us pandas-2 compatible.

@i-am-sijia
Copy link
Contributor Author

I have some updates for the Sharrow runs. Below are three 1-zone benchmarking runs with Sharrow. The one on the top (3/20) used the main branch at 6d817be, the middle (3/28) and bottom (3/29) ones both used this PR with the latest code changes I made last week. There is a lot to discuss here.

  1. The peak memory of the entire model did not reduce much after dropping unused variables, although there were reductions in most components. With the main branch, the memory peak was Mandatory Tour Scheduling (156 GB), and a close runner-up was Write Trip Matrices (155 GB). After dropping unused variables, Mandatory Tour Scheduling reduced to 101 GB, but Write Trip Matrices reduced to 147 GB. So the peak memory usage only reduced from 156 GB to 147 GB.

  2. The two runs after dropping unused variables showed different memory usage patterns, even though they used the same source code. The one on 3/28 showed accumulation of memory in Trip Destination, while the one on 3/29 showed accumulation of memory in Mandatory Tour Scheduling (that caused School Escorting having a high mark to start with). This looks relevant to issue Double check Sharrow memory usage fix (#751) is merged and implemented #816. We have "memory leaks" that seem to show up sometimes but not all times.

  3. The total run time of the 3/28 and 3/29 runs are longer than the 3/20 run, 16.2 hours vs 13.4 hours. This could mean that dropping unused variables might have caused longer run time, even though I did not observe longer run time in the Non Sharrow runs reported week. In the Non Sharrow runs, in addition to a 25% peak memory reduction, dropping unused variable also reduced run time by 10 mins. One possibility is that dropping variables in the Sharrow mode has implications on run time that I have not realized. Another possibility is that the machine needs a reboot as things piled up that slowed it down (although this is a virtual machine with nothing else running).

run time comp sharrow

<style> </style>
Event WSP Sharrow On 3-20 Main WSP Sharrow On 3-28 PR 833 WSP Sharrow On 3-29 PR 833
Max Memory (GB) 156.1 146.8 150.5
mandatory_tour_scheduling 156.1 101.8 109.9
write_trip_matrices 155.0 146.7 147.2
trip_destination 132.8 146.8 130.4
school_escorting 124.8 114.3 150.5
non_mandatory_tour_scheduling 116.6 106.7 88.6
atwork_subtour_scheduling 89.3 43.1 42.0
workplace_location 82.8 72.9 81.2
atwork_subtour_mode_choice 79.6 40.3 37.9
trip_mode_choice 66.6 60.2 61.0
stop_frequency 64.2 62.2 65.1
non_mandatory_tour_destination 61.7 67.9 64.8
vehicle_allocation 60.1 99.1 88.2
write_data_dictionary 59.7 48.3 48.5
trip_scheduling 58.9 50.3 50.7
school_location 58.9 57.9 63.6
trip_purpose_and_destination 55.7 50.7 48.3
atwork_subtour_destination 54.3 51.0 48.0
track_skim_usage 53.2 41.5 41.8
write_tables 52.3 43.6 44.0
finalizing 48.6 41.4 41.7
tour_mode_choice_simulate 47.9 52.9 45.4
joint_tour_destination 47.2 48.0 49.8
non_mandatory_tour_frequency 42.5 50.7 41.7
trip_purpose 39.4 38.1 38.4
joint_tour_scheduling 36.5 36.0 33.0
atwork_subtour_frequency 30.8 30.2 29.3
vehicle_type_choice 28.8 28.7 27.6
joint_tour_participation 28.8 29.9 28.5
cdap_simulate 24.8 24.0 25.5
joint_tour_frequency 24.2 24.3 24.4
free_parking 21.6 23.3 23.5
joint_tour_composition 21.3 21.2 21.2
compute_disaggregate_accessibility 20.7 23.1 21.2
initialize_households 20.4 21.2 22.3
mandatory_tour_frequency 18.5 23.0 23.7
compute_accessibility 12.7 15.5 16.9
auto_ownership_simulate 11.1 12.0 11.8
initialize_landuse 7.1 14.0 11.4
initialize_proto_population 6.4 6.4 6.4
input_checker 1.9 1.6 1.6
preload_injectables 0.3 0.3 0.3
na 0.3 0.3 0.3

@i-am-sijia i-am-sijia marked this pull request as ready for review April 2, 2024 19:05
@i-am-sijia i-am-sijia requested a review from jpn-- April 2, 2024 19:13
@i-am-sijia
Copy link
Contributor Author

i-am-sijia commented Apr 8, 2024

Update RE:

The total run time of the 3/28 and 3/29 runs are longer than the 3/20 run, 16.2 hours vs 13.4 hours.

This seems to be because flow.load time increased.

For example, in the 3/20 run:

18/03/2024 19:27:38.294 - INFO - sharrow - flow exists in library: BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2
18/03/2024 19:27:38.294 - INFO - activitysim.core.flow - completed setting up flow school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums in 0:00:00.077000 
18/03/2024 19:27:38.294 - INFO - activitysim.core.flow - begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
18/03/2024 19:30:05.321 - INFO - activitysim.core.flow - completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:02:27.026987 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
18/03/2024 19:30:05.321 - INFO - activitysim.core.flow - completed apply_flow in 0:02:27.103988 
...
18/03/2024 20:09:15.654 - INFO - activitysim.core.flow - setting up sharrow flow vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
18/03/2024 20:09:15.995 - INFO - sharrow - using existing flow code 736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X
18/03/2024 20:09:16.193 - INFO - activitysim.core.flow - completed setting up flow vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils in 0:00:00.552001 
18/03/2024 20:09:16.193 - INFO - activitysim.core.flow - begin flow_736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X.load vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
18/03/2024 20:13:04.941 - INFO - activitysim.core.flow - completed flow_736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X.load in 0:03:48.747827 vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
18/03/2024 20:13:04.941 - INFO - activitysim.core.flow - completed apply_flow in 0:03:49.299828 
...

In the 3/29 run:

29/03/2024 17:41:59.160 - INFO - sharrow - flow exists in library: BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2
29/03/2024 17:41:59.160 - INFO - activitysim.core.flow - completed setting up flow school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums in 0:00:00.078139 
29/03/2024 17:41:59.160 - INFO - activitysim.core.flow - begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
29/03/2024 17:53:31.208 - INFO - activitysim.core.flow - completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:11:32.047284 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
29/03/2024 17:53:31.208 - INFO - activitysim.core.flow - completed apply_flow in 0:11:32.125423 
...
29/03/2024 19:11:49.595 - INFO - activitysim.core.flow - setting up sharrow flow vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
29/03/2024 19:11:49.924 - INFO - sharrow - using existing flow code 736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X
29/03/2024 19:11:49.955 - INFO - activitysim.core.flow - completed setting up flow vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils in 0:00:00.390580 
29/03/2024 19:11:49.955 - INFO - activitysim.core.flow - begin flow_736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X.load vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
29/03/2024 19:21:27.151 - INFO - activitysim.core.flow - completed flow_736GLJ6FZR4AF6ZTRFF5NB4F2D2MYM7X.load in 0:09:37.196634 vehicle_type_choice.interaction_simulate.interaction_simulate.eval_interaction_utils
29/03/2024 19:21:27.151 - INFO - activitysim.core.flow - completed apply_flow in 0:09:37.587213 
...

Not sure if this is because of recent sharrow updates, or because of recent updates in the way unused columns are dropped.

@jpn--
Copy link
Member

jpn-- commented Apr 11, 2024

I am not sure what is happening that has cause the code to run slower for @i-am-sijia , I am unable to replicate the problem.

I have run the full scale model on my laptop through school location choice (the first problematic model shown above) and got these results:

sharrow commit 7fae9f060b77684b2f309c9ad2ad3d2bf3286239 (sharrow as of 3/28)
activitysim commit e5d9878 (this PR as of 3/28)

[08:20.92] INFO: begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
[09:24.30] INFO: completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:01:03.380284 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
[09:24.30] INFO: completed apply_flow in 0:01:03.416361 

sharrow commit c560b45f0e1cd5ccbb871319b5ccfbcf789debaf (sharrow v2.7, in use 3/20)
activitysim commit 94c4db8 (main branch as of 3/20)

[08:11.76] INFO: begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
[09:12.41] INFO: completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:01:00.648809 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
[09:12.41] INFO: completed apply_flow in 0:01:00.678720 

@jpn--
Copy link
Member

jpn-- commented Apr 16, 2024

I recall we discussed that we would like to have a method to turn this feature off for individual components, either because it is interfering with something (e.g. tracing, estimation mode) or just because it's not working on a particular component (probably due to something weird in the spec). I do see that it turns itself off when tracing or estimation mode is used, which is OK, but I think we still want the capability to turn it off manually if desired.

@i-am-sijia
Copy link
Contributor Author

i-am-sijia commented Apr 16, 2024

I reran the reported "3/20" and "3/29" runs, see Test 1 and Test 2 below. Last time they were run on different machines. This time they are run on the same machine. Good news is that this time I'm not seeing a huge runtime difference. Test 3 uses the same commits as one of Jeff's test, apparently the run time on Win is longer than Mac.

Test 1 (Rerunning the "3/20" run)

Sharrow commit c560b45f0e1cd5ccbb871319b5ccfbcf789debaf (sharrow v2.7)
ActivitySim commit 6d817be (main, as of 3/18)
activitysim-prototype-mtc commit 9ca05ca81a240cc74bfd764b1d859b55dbf597e0 (before school escorting sharrow changes)

16/04/2024 13:41:49.585 - INFO - sharrow - flow exists in library: BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2
16/04/2024 13:41:49.587 - INFO - activitysim.core.flow - completed setting up flow school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums in 0:00:00.114174 
16/04/2024 13:41:49.587 - INFO - activitysim.core.flow - begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
16/04/2024 13:49:11.177 - INFO - activitysim.core.flow - completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:07:21.590254 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
16/04/2024 13:49:11.177 - INFO - activitysim.core.flow - completed apply_flow in 0:07:21.704428

Test 2 (Rerunning the "3/29" run)

Sharrow commit c560b45f0e1cd5ccbb871319b5ccfbcf789debaf (sharrow v2.7)
ActivitySim commit e5d9878 (this PR as of 3/28)
activitysim-prototype-mtc commit 9ca05ca81a240cc74bfd764b1d859b55dbf597e0 (before school escorting sharrow changes)

12/04/2024 20:47:53.356 - INFO - sharrow - flow exists in library: BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2
12/04/2024 20:47:53.356 - INFO - activitysim.core.flow - completed setting up flow school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums in 0:00:00.078128 
12/04/2024 20:47:53.356 - INFO - activitysim.core.flow - begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
12/04/2024 20:56:44.189 - INFO - activitysim.core.flow - completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:08:50.832949 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
12/04/2024 20:56:44.189 - INFO - activitysim.core.flow - completed apply_flow in 0:08:50.911078

Test 3 (use the latest sharrow)

Sharrow commit 7fae9f060b77684b2f309c9ad2ad3d2bf3286239 (sharrow v2.8.2 as of 3/28)
ActivitySim commit e5d9878 (this PR as of 3/28)
activitysim-prototype-mtc commit 9ca05ca81a240cc74bfd764b1d859b55dbf597e0 (before school escorting sharrow changes)

15/04/2024 13:55:03.917 - INFO - sharrow - flow exists in library: BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2
15/04/2024 13:55:03.917 - INFO - activitysim.core.flow - completed setting up flow school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums in 0:00:00.082882 
15/04/2024 13:55:03.917 - INFO - activitysim.core.flow - begin flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
15/04/2024 14:01:53.135 - INFO - activitysim.core.flow - completed flow_BJFBKLAOJGCDYZF4MKEWTG744CT3JZF2.load in 0:06:49.217218 school_location.i1.logsums.gradeschool.compute_logsums.eval_nl_logsums
15/04/2024 14:01:53.135 - INFO - activitysim.core.flow - completed apply_flow in 0:06:49.300100 

@i-am-sijia
Copy link
Contributor Author

i-am-sijia commented Apr 17, 2024

I recall we discussed that we would like to have a method to turn this feature off for individual components, either because it is interfering with something (e.g. tracing, estimation mode) or just because it's not working on a particular component (probably due to something weird in the spec). I do see that it turns itself off when tracing or estimation mode is used, which is OK, but I think we still want the capability to turn it off manually if desired.

@jpn-- , I made the key code changes needed for turning this feature on and off for individual components, I haven't pushed them. It's essentially passing a Boolean drop_unused_columns from component model settings into simple_simulate, interaction_simulate, interaction_sample etc. I modified almost all component .py (like auto_ownership.py) to feed in this setting.

Then I paused and read your latest comment on #824. Since you are generalizing the sharrow_settings to compute_settings and I checked that they are consumed at the same place where drop_unused_columns are required, it may make sense to include drop_unused_columns in compute_settings, or I can create a sibling of compute_settings to host drop_unused_columns and any other settings that might be created in the future. Thoughts?

@jpn--
Copy link
Member

jpn-- commented Apr 18, 2024

it may make sense to include drop_unused_columns in compute_settings

I like this idea. Let's finish the review/merge of #824 and then it should be easy to update this PR and put this straight into the compute_settings structure.

@i-am-sijia
Copy link
Contributor Author

@jpn-- , I implemented drop_unused_columns in compute_settings, cherry-picked the parking location changes in #849 and the protect additional variables in https://github.com/camsys/activitysim/commit/574ee0eb444e0e97c5b2ac8c9215ed9df55bd53d, so that everything is included in this PR for efficient review and merge.

I tested setting compute_settings.drop_unused_columns to false in auto ownership, non-mandatory tour destination and scheduling, school escorting, and trip mode choice. It worked as expected.

@jpn-- jpn-- merged commit 239f415 into ActivitySim:main Apr 21, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants