-
Notifications
You must be signed in to change notification settings - Fork 0
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
class semantics approach #37
Comments
Hey this is off to a great start! I added comments throughout the files you created and just pushed the changes. I have been trying to get it to run, and it looks close, but couldn't quite finish debugging it. I'm curious to know if you can get it to run. |
Glad to hear you think it's off to a great start! I only really wrote the code to get the idea out there, I never actually tried it, so that could explain the bugs you're getting—I'm sure there's bits of crucial machinery from the old approach that haven't been copy/pasted in here. I saw your comments and based off them I think it is possible to make an operator class to make them more modular—I'll give that a try. Update after trying to make the class: making the class depends on which methods will be shared by all operators and whether there will be any special methods/attributes that a specific operator would have, if any (the question of how a frame would access those is relevant here I think) |
Having done a bit more thinking about the class semantics, here are some classes that I think will be important to define:
The hope with organizing things this way (subject to changes) is that the classes 1 and 5 could be suppressed, and only 2-4 exposed. By defining 2-4, one could then rely on the general functionality provided by 1 and 5 in order to set up a semantics. We could also include various helper functions that would also be suppressed. The thought would be that the model checker will come with some default semantics, but running I imagine this can all be improved, but hopefully provides a reasonable overview. |
Yeah that all sounds really good and I think gets around the previous problems we were having in the old attempt—it should be possible (I hope). I agree that it would be good to hide 1 and 5, ideally users get less objects to customize just to keep things simple, but get lots of room to customize what they do get. I can give this all a try before tomorrow's meeting to hopefully be able to discuss any implementation/design challenges then! |
Great meeting today! I created these notes which goes over the high level architecture with minor revisions and also added a plan to the TODO.md. Although we may need to return at times to make tweaks, I think we have about finished with the architecture phase and on to implementation. I'll leave this issue open for high level topics. |
Although it is still a ways off, I'm starting to think about the main function that will compose the various classes.
In (1), I'm wondering what the best way to pass Operator classes into ModelSetup while permitting the number of Operator classes to remain flexible since some semantic systems may require more than others. If this seems like a reasonable first pass, I'll add it to the TODOs and we can revise it as need be later. |
Hey Ben, sounds good! So far no issues in trying to implement this. One small question though—right now in model_checker there is no ModelStructure class (only StateSpace). For the new approach were you thinking that what is currently StateSpace becomes ModelStructure or to have both? |
ModelStructure might be a better choice so as not to presume to much, but both names refer to the same thing. |
Got it, sounds good! |
Just looked at the new modules. They are looking really good! I added some comments and questions throughout. One minor issue that is relevant to the overall architecture is that there are a lot of arguments for |
We concluded that defining an Input class can be added once we start applications if it feels like it would improve the conceptual hygiene. We also discussed grouping the semantics, operators, and Proposition objects together so that users could compare different combinations. We can return to this once we apply the new semantics. |
I moved |
Looks good to me! I just went through and commented some things/removed old comments/answered comments, let me know what you think particularly about the comment on the Proposition class. |
Just reviewed comments. I lke the parent proposition class idea. I renamed things so that the user defined subclass reads: Defined(Proposition). Maybe not the best name, but seems reasonable enough. Happy to change latter if we think of a better name. I also added some linter comments about shortening the dunder methods in the Operator class. |
I have been messing with the hash function in Proposition. Here is what it looks like now with attempts commented below:
I also initialized a name so that proposition instances could be added to the
This allowed me to change the Defined(Proposition) subclass init to the following:
It seems to be finding models and printed the following when I ran
This seems right given that |
Nice! I actually was also playing around with the hash function and found a way to move it to the parent class (so that users don't have to worry about it). I'll push to incorporate that with the changes you've made! |
Hopefully I didn't clobber your changes, but resolved some merge conflicts. It seems to be working. |
The |
Sounds good—and actually thinking about it we don't even need the |
Oh got it. Seems to all be working though my linter is raising an error for the except clause in the Proposition
This also seems to work, so I commented out the try/except. Not really sure if the above does what we want. |
The except clauses are necessary if you want to add Proposition objects to a set. The following code in
I'm not entirely clear on why this is (since the parent class has a hash method) but it looks like if you redefine |
Changing 'x' to 'self' seems to work. Do propositions need to be added to a set if we are using a dictionary? I'm still not totally clear about what the try/except thing is doing but it is working. Also not sure if I turned the I have also been moving helper functions from |
Propositions don't need to be added to a set if we are using a dictionary, but if we want to let the user define equality then we need to define hash as well—I gave some thoughts in a couple comments in the code. I think it would be better to have it as its own function—and on the note of moving things into |
Just looked at your comments and I think that makes sense. It seems important to think about all this especially in anticipation of setting up the next hyperintensional layer of the semantics. Perhaps the best thing to do here is to open an issue to document where we stand and what remains. We can then come back to this once the semantics for CFs has been implemented. I'll cut your comments into a new issue. |
Implemented a way to see if DerivedOperators are defined in terms of each other (and got rid of the error message with the default arg |
I started a new branch with some work on the class semantics. It has two new files:
class_semantics_playground.py
andclass_semantics_example.py
. Both have docstrings explaining some things and problems I ran into. Something I didn't mention there but is worth saying is there is a new class called aFrame
class—it effectively replaces the semantics; if there is a better name for it by all means feel free to change it (and anything else).The text was updated successfully, but these errors were encountered: