Conversation
|
Not urgent, but please fix the title of the PR at some point to start with the required |
chenson2018
left a comment
There was a problem hiding this comment.
This looks much better! The theory now all lines up, there are some additional small style/technical issues I have pointed out below. Please also see the failures in CI now that it has made it to the linting step.
Co-authored-by: Chris Henson <46805207+chenson2018@users.noreply.github.com>
| We first define what is denoted Tran* in [WinskelNielsen1995]: the extension of | ||
| a transition relation with idle transitions. | ||
| -/ | ||
| def lift (trans : State → Label → State → Prop) : State → (Option Label) → State → Prop := |
There was a problem hiding this comment.
This is currently named Cslib.lift, which still isn't a great name/location combo. I'd say LTS.lift, except this is unbundled which makes that an odd name. Deferring to @fmontesi on this.
There was a problem hiding this comment.
Yes, we should namespace it under LTS. But lift looks too generic a name to me, this is doing something very specific, i.e., adding so-called 'idle' self-transitions. I'd also suggest to make it transform an LTS, so to enable stating properties about it without relying on knowledge of how LTS is defined.
How about LTS.withIdle or LTS.withSelfLoops or something like that?
In general, it would also make sense to move such operations on unbundled LTSs to other files, outside of category theory -- but we don't necessarily need to do this now. I see some operations in Winskel and Nielsen (like relabelling) that make sense to offer even to developments not interested in category theory.
There was a problem hiding this comment.
withIdle seems fine, but it is maybe a bit odd for this to be in the LTS namespace when it doesn't actually include an LTS in the signature.
chenson2018
left a comment
There was a problem hiding this comment.
Please see my naming comment above, but otherwise things look in pretty good shape to me.
(I pushed a few small commits, the most important of which is probably using some API from Mathlib.Control instead of breaking down notation with change)
This PR resolves #387.