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

Rename / remove dense and sparse variables and refactor #103

Merged
merged 16 commits into from
Feb 13, 2025
Merged

Conversation

timmens
Copy link
Member

@timmens timmens commented Feb 10, 2025

Future PRs:

  • Update explanation notebooks and run again in GHA
  • Currently we have discrete_problem._determine_dense_discrete_choice_axes and simulate.determine_discrete_dense_choice_axes: They look similar, and do similar things, but not exactly -> Combine these with an argument like purpose: Literal["solution", "simulation"].

@timmens timmens requested a review from hmgaudecker February 12, 2025 17:23
@timmens timmens changed the title Remove sparse variables and refactor Rename / remove dense and sparse variables and refactor Feb 12, 2025
Copy link
Member

@hmgaudecker hmgaudecker left a comment

Choose a reason for hiding this comment

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

Very neat!

I think type annotations throughout would really help. Especially getting rid of None instances where empty containers will just do the trick and allow simplifying the code. No need to include here, but probably a good one to tackle soon.

@timmens
Copy link
Member Author

timmens commented Feb 13, 2025

As discussed, we will further split the code-base into parts that are relevant only for simulation or solution. For this we will also split spacemap into two functions; therefore we won't resolve the comments on dispatchers.py here, but in a separate PR.

Copy link
Member

@hmgaudecker hmgaudecker left a comment

Choose a reason for hiding this comment

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

Sweet!

Copy link
Member

@hmgaudecker hmgaudecker left a comment

Choose a reason for hiding this comment

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

👍

@timmens timmens merged commit eb1e5bf into main Feb 13, 2025
6 checks passed
@timmens timmens deleted the refactor-sparse branch February 13, 2025 15:38
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.

2 participants