cxbrooks / test

Second test for bugzilla to git
0 stars 0 forks source link

Removal of a port type declaration does not propagate to the model #316

Closed cxbrooks closed 11 years ago

cxbrooks commented 12 years ago

Note: the issue was created automatically with bugzilla2github tool

Original bug ID: BZ#518 From: @lhstrh Reported version: 9.1.devel CC: pt-dev@chess.eecs.berkeley.edu

cxbrooks commented 12 years ago

Thanks to Hogan, D. (GE Energy) for pointing this out:

After setting the input field 'Type' in the 'Configure Ports'-panel to "" (empty) and hitting the 'Commit' button, a MoML change request is issued that looks like this: ''.

This request is parsed by the MoMLParser and accordingly, the attribute is removed from the MoML representation. However, unlike a regular type change where elementName.equals("property") (see MoMLParser:3099) rather than a removal where elementName.equals("deleteProperty") (see MoMLParser:2490), the running model doesn't get notified that the previous type is now invalid.

Note that upon a regular property change MoMLParser.startElement() calls _handlePropertyElement(), which then adds the parameter to _paramsToParse after which it is retrieved in handler.endDocument() which then calls param.validate().

If the input field is set to "", the attribute only gets removed from the MoML representation, but the model is unaware of this. Currently, the old type persists. The only way to overcome this problem is to (save and) re-open the model, which forces Virgel to entirely parse the MoML description and build up the model from scratch, which will set the type in question to UNKNOWN.

The desired behavior would be that the requested type changes immediately to UNKNOWN.

cxbrooks commented 12 years ago

As a quick fix I added on PortConfigurerDialog.java:582

if (tableValue.equals("")) { tiop.setTypeEquals(BaseType.UNKNOWN); }

This bypasses the MoMLParser and sets the type directly. The behavior is now as desired, but in the end it might be better to adapt the parser itself.

cxbrooks commented 12 years ago

Here's my post to ptolemy-hackers. I think there is a bug in that Manager.resolveTypes() is failing to redo type resolution

--start--

I took a look at the ptolemy/actor/gui/PortConfigurerDialog.java and it looks like when I use the dialog to change the type from double to nothing, then the following moml request is issued:

Looking at the MoML for the RecordDisassembler, I see that the _type property is not present: The problem is that when we delete the _type property, we need to tell the type system that any cached type data is invalid. One way to do this is to find the Director and call invalidateResolvedTypes. I hacked in some code to do this, but then realized that problem is that Manager.resolveTypes() was being called even without my hacks. So, it seems like the problem is that Manager.resolveTypes() is incorrectly holding on to some data. The way to replicate the problem is to put a breakpoint at Manager.resolveTypes(), follow the steps below and note that after setting the type to blank, resolveTypes() is still called. Marten, could you take a look? BTW - Edward added an attribute called ShowTypes that when dragged into a model shows the types. In the left hand actor pane, see Utilities -> Analysis -> ShowTypes. _Christopher On 7/18/12 2:39 PM, Hogan, D. (GE Energy) wrote: > Attached is a model file demonstrating this issue. I'm trying to > understand why this happens and if it should happen. This is the setup > of the moml file in case the attachment gets stripped: > > * SDF director > * Const actor with { foo = 5, bar = "nil" } as the value and > firing count set to 1. > * RecordDisassembler actor with input port connected to the const > actor. > o Added a new output parameter called "foo" and leave the type > undeclared > * AddSubtract actor with the add port connected to > RecordDisassembler's "foo" output port. > * Display actor connected to the AddSubtract output > > If I change AddSubtract.plus' type to double, run it and change the type > back to an empty string, it is still resolved to double. All of the > associated input and output ports are resolved to double even if foo is > an int. No actor has a port type of double so why is it still resolved > to double? The setup of the actors in the GUI is identical to when it > was first opened, but the result is different (5.0 vs. 5 when foo=5). > > If I save the file at this point, it reverts back to the old behavior > where the types are resolved properly. AddSubtract.plus' type is still > empty but setting foo=5 displays 5. This is strange when I get > different answers before and after the save. > > Why does changing the type back to an empty string keep the double type > resolution until I reload the file? Shouldn't setting the type to an > empty string be equivalent to the default undeclared? Or shouldn't it > reject my change to an empty string since it's effectively still double > until I reload the file? > > I think I should be able to open that file, run it (get 5 as output), > change AddSubtract.plus' type to double, run it (get 5.0 as output), > change AddSubtract.plus' type back to an empty string, run it and get 5 > as the output. --end--
cxbrooks commented 12 years ago

Created attachment 48 ModelThatFailsToTypeCheckAfterChangingPortType

Attached file: foo_test.moml (application/xml, 4269 bytes) Description: ModelThatFailsToTypeCheckAfterChangingPortType

cxbrooks commented 11 years ago

Could not reproduce; changing the type of RecordDisassembler.foo into double and then running the model results in a double output, then removing the type annotation and running the model again results in the original integer output. This looks like something that I ran into and fixed earlier, not being aware of this ticket. Considering this problem fixed.

cxbrooks commented 11 years ago

I can't reproduce this, I believe it was fixed.