EnergyInnovation / eps-us

Energy Policy Simulator - United States
GNU General Public License v3.0
22 stars 7 forks source link

Maybe consolidate "Vehicle Type" and "Cargo Type" subscripts #174

Closed jrissman closed 2 years ago

jrissman commented 2 years ago

There is no benefit to having a separate "Cargo Type" subscript, and there are a number of drawbacks, particularly as we no longer wish to use passenger and freight versions of aircraft and ships (per issue #153), and we never used freight motorbikes (often repurposing it to another vehicle type, such as "three-wheelers" in India). We could extend the "Vehicle Type" subscript to handle all the vehicle types now being handled by Vehicle Type and Cargo Type (e.g. a 1x12 matrix instead of a 2x6 matrix).

As of now (7/8/21), EPS version 3.3.0 contains only 40 variables that use four subscripts. (No variables use more than four subscripts.) All 40 of these variables include the "Cargo Type" subscript, so no variable in the EPS would have more than three subscripts if we merge "Cargo Type" into "Vehicle Type."

The number of subscripts per variable is relevant because SDEverywhere, a tool for transcoding Vensim models into WebAssembly, currently only supports models that contain variables using no more than three subscript dimensions on a single variable. We could not live with this restriction forever, as we will need to add four (or more) subscripts to specific variables in the future. However, it would enable EPS 3.3.0 to more easily work with SDEverywhere and could be used as a testing platform to help SDEverywhere add support for higher numbers of subscript dimensions per variable.

jrissman commented 2 years ago

I decided to take a stab at this today, thinking it might only take an hour or two. It is turning out to be much more difficult than I anticipated. While only 40 variables contain 4 subscripts, many variables use "Cargo Type" or refer to variables using "Cargo Type." The code in the .mdl file needed to be updated in around 1500 different places. It would have been impractical to do this by hand, so I did most of the updates programmatically, and did the most difficult 400 or so updates by hand.

Unfortunately, that method of updating code is prone to causing bugs, and it took me a while to debug it and get the model to run. After about 8+ hours of work, the model runs, but it's currently producing nonsensical results for the transportation sector. So I would still need to go and find where the equations now don't make sense and fix them.

I think I probably would not have tried this if I'd known how difficult it would be at the beginning. It's almost done now, so I might try to finish it off. But it doesn't actually seem like it's adding much value. It reduces the number of .csv input files a bit and makes some of the equations a bit more readable, and it saves us a subscript on the transportation sector policy levers (where we are capped at two subscripts in the web interface), but these weren't important limits for us before.

On the downside, we're probably adding just as many equations as we remove, because for every place I can remove the "passenger" and "freight" versions of an equation (usually when loading input data), there's some other place where we only had input data by vehicle type (not by Cargo Type), so I need to add six new equations to read the same input files each twice (for example, once for passenger LDVs and once for freight LDVs). So in this specific sense, at least, it isn't really simplifying the model.

It seems like the value is really minimal aside from helping with SDEverywhere development. So much so that I'm not sure if I should even merge this work (all done in branch 174) into the main develop branch. Todd can use an EPS version in a side-branch for testing and developing SDEverywhere just as easily as he can use the main branch. So the SDEverywhere consideration is irrelevant to the question of whether to finish and merge this work into the main branch, or to just leave it in the side branch forever.

jrissman commented 2 years ago

Commit 6c785f9 fixes the problem that was causing nonsensical results. The results in this branch look fine now . If I want to finish this, the last steps are to update WebAppData and the policy packages' lever settings. It wouldn't be much more time to finish those last little bits, but I'm still on the fence about whether to get rid of or to keep the Cargo Type subscript in the main development line.

robbieorvis commented 2 years ago

I’d prefer not for now given the amount of data reprocessing required

Sent from my iPhone

On Jul 12, 2021, at 5:06 PM, Jeff Rissman @.***> wrote:



Commit 6c785f9https://github.com/Energy-Innovation/eps-us/commit/6c785f91dc8a57fa73f7f9af8a8a18eea1e9f195 fixes the problem that was causing nonsensical results. The results in this branch look fine now . If I want to finish this, the last steps are to update WebAppData and the policy packages' lever settings. It wouldn't be much more time to finish those last little bits, but I'm still on the fence about whether to get rid of or to keep the Cargo Type subscript in the main development line.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/Energy-Innovation/eps-us/issues/174#issuecomment-878595158, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK5N6SO6NRDNRABQRXBVGCDTXNKMVANCNFSM5AB4WVKA.

jrissman commented 2 years ago

"For now" would mean forever, in this case. If we make almost any other change to the main EPS model before we merge in this branch, it will become impossible to merge in the work I've done in branch 174. There is no way I'm going to repeat the difficult and meticulous work I just did (and I doubt any other person could do it, at least not without great expenditure of time and introducing many bugs). So we must recognize that this is the one and only opportunity to ever have the Cargo Type and Vehicle Type subscripts merged.

The amount of input data reformatting is minor compared to the amount of work required in the .mdl file to support this merge (and I already did the data reformatting for the U.S. model). So we should be making this decision primarily on the basis of how we want the EPS model to be forever. In other words, what would make for the best computer model? We can discuss on the next EPS call, if you like. I think either to keep the work or to discard it are very reasonable decisions. I just want to make sure we consider all relevant factors when making our decision.

jrissman commented 2 years ago

Pros of merging Vehicle Type and Cargo Type:

Cons of merging Vehicle Type and Cargo Type:

You're right that input data reformatting is not trivial given the other pros and cons and the number of preexisting EPS adaptations that might want to go to 3.3.0 or later. Probably all of them eventually would.

I suppose I'm leaning toward discarding the work done in branch 174. I should have been better at estimating the labor, pros, and cons beforehand, but sometimes it's difficult to see these things clearly in advance.

robbieorvis commented 2 years ago

Thanks, Jeff.

Happy to discuss this more on our team call.

For the reasons you outlined, I am also disinclined, even if it means losing the work (which would be a shame, but happens often in EPS programming). I don’t see significant value add and the hurdles are significant (more below).

To the latter point: we had discussed 3.3.0 being an incremental update to get a few things out ahead of a bunch of new models prior to beginning more significant structural work. Adding this into 3.3.0 creates a new significant hurdle because of the input data reformatting requirements and the current status of development.

This change would require a lot of work from our team to update the EU model as a starting point. It might also preclude us from moving the state models to 3.3.0 because RMI has been really hard at work on the code and are trying to wind down that work (and have explicitly told us they need to). Requiring a change in format to many input data files is likely too high a hurdle at this stage of the project for them to agree. Then there are the other international models that are partners are working on, and having to redo the formatting for those (two China models, Mexico, KSA, India, and Brazil) is a substantial hurdle.

So, while it’s a shame to lose something we invested time into, I think it creates significant challenges (and it’s not totally lost; as you point out Todd can use this version for testing SDEverywhere).

Anyway, that’s my two cents, but let’s chat more during the EPS check-in.

-R


Robbie Orvis Senior Director of Energy Policy Design +1 415-799-2171 98 Battery Street, Suite 202 San Francisco, CA 94111 www.energyinnovation.orghttp://www.energyinnovation.org/ @.***D7780A.3B8384C0]

From: Jeff Rissman @.> Sent: Monday, July 12, 2021 5:42 PM To: Energy-Innovation/eps-us @.> Cc: Robbie Orvis @.>; Comment @.> Subject: Re: [Energy-Innovation/eps-us] Maybe consolidate "Vehicle Type" and "Cargo Type" subscripts (#174)

Pros of merging Vehicle Type and Cargo Type:

Cons of merging Vehicle Type and Cargo Type:

You're right that input data reformatting is not trivial given the other pros and cons and the number of preexisting EPS adaptations that might want to go to 3.3.0 or later. Probably all of them eventually would.

I suppose I'm leaning toward discarding the work done in branch 174. I should have been better at estimating the labor, pros, and cons beforehand, but sometimes it's difficult to see these things clearly in advance.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/Energy-Innovation/eps-us/issues/174#issuecomment-878616679, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AK5N6SM4BELJWV7VC2HNF5LTXNORZANCNFSM5AB4WVKA.

jrissman commented 2 years ago

I think you're right. Let's leave this in branch 174 permanently, as it doesn't really add value, and you're right about how we want 3.3.0 to have minimal input data changes so as to be an easy upgrade path for existing models. The work I did will still be useful because Todd can use branch 174 to get SDEverywhere working in all respects other than support for 4+ subscripts. (And I did the work over the weekend knowing it was speculative and there was a chance it might never be integrated into the main development line of the model, so the fact that it looks like we're not going to merge it in isn't completely unexpected.)

jrissman commented 2 years ago

I'm going to close this issue, since I don't think any more work will be necessary on it, as we're not merging it into the main "develop" line. However, I will note here the remaining items to do, in case in the future this branch needs to be turned into a final, fully-working EPS model:

All together, these final updates probably would take maybe 1 hour, so it isn't a huge lift if we do need to do the last steps to create a fully done and working version for SDE testing/development.