-
Notifications
You must be signed in to change notification settings - Fork 42
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
Wishlist for Ciw 3.0 #206
Comments
@drvinceknight @11michalis11 Let me know your thoughts / suggestions. |
May I add a couple of ideas?
Hope it helps |
HI @lec00q thank you so much for the suggestions! I believe points 2) and 5) can be done already in some way:
I love the idea of 3) and 4). Do you have any examples of this so that I can further understand what is meant here? I think 1) is quite difficult. When implementing reneging we found it difficult to clearly well define a state-dependent reneging mechanism without falsely multiply sampling, and so changing the probability distributions. I would welcome a further discussion on this though. |
I'm happy to PR this if you'd like me to @geraintpalmer |
@drvinceknight that would be fantastic thank you |
This was discussed at CiwPython#206. Note that this adds a script to `docs/_static`. This is because I do not believe the parallel processing can be doctested.
For # 4 we can actually use immutable types that have an ordering. See Can Ciw Use Tuples For Class IDs?. Personally, I like this flexibility. |
Thanks @galenseilis this is a nice idea. I initially thought to keep customer classes as strings or integers, in order to make the data records easier to read with pandas, and reading/writing from file. Do you see any issue with this? |
I think that not making any further changes to customer classes is desirable for my use cases. Using strings or integer works in many cases, and it is also compatible with using tuples depending on the project. A column of tuples can be "exploded" into multiple columns using pandas, so I am not particularly concerned about that. Overall, I like the current state. |
Just an afterthought about Pandas & Ciw: I think it is good to keep Pandas out of Ciw. Returning a dictionary as per the current state is good enough. Partly because there are other dataframe tools (e.g. polars), but also because Pandas is not compatible with PyPy. On larger simulations PyPy can provide some easy wins on performance even if it means avoiding certain packages (e.g. SciPy, Pandas, SKLearn). |
The possibility of a 3.0 release opens up some possibilities, allowing some big changes that are not back-compatible, and some smaller internal changes. Below is a list of possible features or changes that might appear. Please use the comments below to discuss.
pyYaml
does this for us and better)pandas
does this for us and better. Ciw records are already lists of NamedTuples with are easily compatible with pandas)DataRecords
(exactly the same as how reneging and interrupted services are, for consistency)"Class 1",
just index by the number1
)?'class_names'
and'node_names'
in the Network object, so that the data records show these. This might make Ciw records more readable.current_time
/next_event_time
internally (I thinkcurrent_time
is better)Decimal(3.2) + 4.1 = Decimal(7.3)
, so maybe no need for theincrement_time
thingimport_params.py
(especially in terms of routing functions)ciw.dists.NoArrivals()
withNone
(this is how reneging and dynamic classes handle no distributions)Q = ciw.Simulation(N, attributes_to_record=['successful_service', 'age'])
(see error rate #205 (comment) for how this is currently handled)The text was updated successfully, but these errors were encountered: