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

fix/remove-loaded-empty -- removes _loaded and _empty fields from rail vehicle #81

Merged
merged 71 commits into from
Sep 9, 2024

Conversation

calbaker
Copy link
Collaborator

@calbaker calbaker commented Jul 2, 2024

Highlights

  • rail_vehicles: Vec<RailVehicle> is now an input to TrainConfig::new()
  • for empty and loaded rail vehicles, you must provide them as two separate vehicles types
  • rail_vehicle is no longer needed as input to make_.*_train_sim
  • make_.*_train_sim_parts is now a thing for getting intermediate objects for inspection

TODO

  • apply Mass trait to train stuff, where appropriate -- @calbaker
  • make some tests that use the demos to compare against past results, perhaps using environment variables -- @calbaker
  • update the pyi file -- @FDsteven

@calbaker calbaker requested a review from mbbruch July 2, 2024 22:41
@calbaker calbaker self-assigned this Jul 2, 2024
@calbaker calbaker changed the title Fix/remove loaded empty removes _loaded and _empty fields from rail vehicle Jul 2, 2024
@calbaker calbaker marked this pull request as draft July 2, 2024 22:41
calbaker added 19 commits July 3, 2024 15:04
python unit tests pass
some functional tests fail
Runing `python speed_limit_train_sim_demo.py` yields:
Traceback (most recent call last):
  File "/Users/cbaker2/Documents/github/altrios/python/altrios/demos/speed_limit_train_sim_demo.py", line 77, in <module>
    train_sim: alt.SpeedLimitTrainSim = tsb.make_speed_limit_train_sim(
RuntimeError: [altrios-core/src/train/train_config.rs:644]

Caused by:
    Extra values in `n_cars_by_type` that are not in `car_type`: ["Manifest_Empty", "stivelrlydt"]
running `python speed_limit_simple_corr_demo.py` yields:
Traceback (most recent call last):
  File "/Users/cbaker2/Documents/github/altrios/python/altrios/demos/speed_limit_simple_corr_demo.py", line 73, in <module>
    train_sim: alt.SetSpeedTrainSim = tsb.make_speed_limit_train_sim(
RuntimeError: [altrios-core/src/train/train_config.rs:647]

Caused by:
    [altrios-core/src/train/train_config.rs:163]
    Expected `self.n_cars_by_type` to contain 'Manifest_Empty'
bail!("`set_mass` is not enabled for `RailVehicle`")
}

fn derived_mass(&self) -> anyhow::Result<Option<si::Mass>> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we size locomotive consists in the train planner, we also include axle_count * mass_extra_per_axle_kilograms when estimating mass. Just flagging to confirm we don't need to harmonize across the code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbbruch, that should be correct, and right now, I don't think there is a cleaner way to do this. I think there is room for improvement that we should discuss, but I think that can come later.

Copy link
Collaborator

@mbbruch mbbruch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment for one thing to double-check in rail_vehicle.rs but otherwise all looks good to me.

@calbaker calbaker marked this pull request as ready for review September 9, 2024 16:30
@calbaker calbaker merged commit 14700b4 into main Sep 9, 2024
3 of 4 checks passed
@calbaker calbaker deleted the fix/remove-loaded-empty branch September 9, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants