It looks like you're new here. If you want to get involved, click one of these buttons!
I tested this on KLayout 0.24-python-eval under Windows 7.
To illustrate the issue(s), I have created a PCell which is a prototype of a via/contact cell. The idea is that it has two ways of specifying its size; one would be to define the number of contacts (and the size follows) and the other would be to specify the size (and the number of contacts follow). Here is the code:
class MyPCell < RBA::PCellDeclarationHelper
def initialize
super
param(:size, TypeInt, "Size", default: 100)
param(:old_size, TypeInt, "Size", default: 100, hidden: true)
param(:contacts, TypeInt, "Contacts", default: 2)
param(:old_contacts, TypeInt, "Contacts", default: 2, hidden: true)
end
def coerce_parameters_impl
puts "Entering coerce_parameters_impl"
if size != old_size
puts " Changing size from #{old_size} to #{size}."
set_old_size(size)
set_contacts(size / 50)
set_old_contacts(contacts)
end
if contacts != old_contacts
puts " Changing contacts from #{old_contacts} to #{contacts}."
set_contacts(contacts)
set_size(contacts * 50)
set_old_size(size)
end
end
def produce_impl
begin
puts "Entering produce_impl with size #{size} and #{contacts} contacts"
metal_layer_idx = layout.layer(RBA::LayerInfo.new(1, 0))
contact_layer_idx = layout.layer(RBA::LayerInfo.new(2, 0))
cell.shapes(metal_layer_idx).insert(RBA::Box.new(-size, -size, size, size))
contacts.times do |yi|
cyi = 2 * yi - contacts + 1
contacts.times do |xi|
cxi = 2 * xi - contacts + 1
cb = RBA::Box.new(cxi * 50 - 12, cyi * 50 - 12, cxi * 50 + 12, cyi * 50 + 12)
cell.shapes(metal_layer_idx).insert(cb)
end
end
rescue
abort("#{$!}\n#{$!.backtrace.join("\n")}")
end
end
end
class MyPCellLib < RBA::Library
def initialize(pcell_classes)
pcell_classes.each do |pcell_class|
pcell_obj = pcell_class.new
layout.register_pcell(pcell_class.name.to_str.split(':')[-1], pcell_obj)
end
register(self.class.name.split(':')[-1])
end
end
MyPCellLib.new([MyPCell])
As you can see, I added some code to log the coerce_parameters_impl
and produce_impl
calls and track the parameter changes and encountered a couple of things I didn't expect:
When changing one of the parameters in the GUI and clicking Apply, there is not only one, but three calls to coerce_parameters_impl
. This in itself probably doesn't really matter (other than some wasted CPU cycles) but what I found surprising is the parameter change log that it printed:
Entering coerce_parameters_impl
Changing contacts from 2 to 3.
Entering coerce_parameters_impl
Changing contacts from 2 to 3.
Entering coerce_parameters_impl
Changing contacts from 2 to 3.
There seems to be something strange going on, in that the parameter change happens multiple times, as if there were three instances of the MyPCell
class which need to be all updated (their contacts
change from 2 to 3). Again, this in itself wouldn't worry me too much if it wasn't for the following:
Sometimes, the parameters don't update properly. Changing the contacts
and size
parameters randomly, I ended up with cases where the size
didn't follow changes to the contacts
and vice versa, causing incorrect parameter combinations such as too many or too few contacts for a given size. This coincides with three calls to coerce_parameters_impl
which don't recognise any parameter changes even though I certainly changed one of them. Here is a log of such a case:
Entering coerce_parameters_impl
Changing contacts from 2 to 1.
Entering coerce_parameters_impl
Changing contacts from 2 to 1.
Entering coerce_parameters_impl
Changing contacts from 2 to 1.
Entering produce_impl with size 50 and 1 contacts
Entering coerce_parameters_impl
Entering coerce_parameters_impl
Entering coerce_parameters_impl
Entering produce_impl with size 50 and 2 contacts
The first produce_impl
call generated a correct layout (I had changed the # of contacts from 2 to 1) while the second one didn't. The last 3 coerce_parameters_impl
calls didn't recognise that I had changed the # of contacts from 1 to 2 again, while the following produce_impl
call then used 2 contacts nevertheless.
Sometimes, changing the parameters repeatedly (clicking Apply every time) gets the GUI in a state where it doesn't accept changes any more: when the Apply button is clicked, they "jump" back to what they were before. A solution to this is to simply close the GUI and re-open it.
When I am talking about the GUI here, I mean either the dialog that comes up when clicking the Instance button or the one which shows up when pressing the q
button (the Object Properties window).
Since I am fairly new to PCells, it also could be my code causing all this behaviour (I re-used some code from this post), so any suggestions on how to improve it would be more than welcome.
Thanks, Chris
Comments
Hi Chris,
KLayout does not have a state model for PCell's - they can be cached or in partially updated state. In other words: you cannot assume a certain history of events. KLayout only guarantees that before produce_impl is called, coerce_parameters_impl is called to bring the parameters into a consistent state. Sometimes, coerce_parameters_impl is called to give the user interface a chance to update dependent parameters. In general, produce_impl must not change parameters - only coerce_parameters_impl can do so.
The intended use case of "coerce_parameters_impl" is to compute dependent parameters, for example to display a computed value (like the character height of the text in BASIC.TEXT).
That works well when you implement a design of the following kind:
In this design there is a forward dependency only: computed parameters depend on free parameters but not the other way. Such a design is safe with respect to state changes, since it coerce_parameters_impl will only update, not feed back changes.
You can use coerce_parameters_impl to track changes, if you're careful. For example, the circle sample does so to figure out whether the handle was moved or the radius parameter was changed in the user interface. If you do, be careful to always leave the object in a consistent state. I your case this means: always leave the object in a state where old_contacts is equal to contacts and old_size is equal to size. I guess this solution works more reliable:
The if conditions are more restrictive making sure that they are only executed when one parameter changes. I assume that if both are changed there is some intention and the script should not make a decision which one to give priority.
An alternative to the scheme above is to provide a check box (boolean) saying "size (false) or count (true)" and one entry field specifying either size or count depending on the choice. To give feedback you could provide two readonly parameters displaying the computed size and contact count. This concept follows the scheme described above and should work flawlessly.
Matthias
Hi Matthias,
Thank you for your great support, I appreciate the time and effort you put into this and hope to be able to return something valuable to this community.
I do understand that by having two interdependent parameters, I am probably moving towards the limits of what the PCells were designed for, but your
Basic.CIRCLE
PCell made me think that this should still be within those limits :-)When I first looked into the RBA::PCellDeclarationHelper source code, I didn't quite understand why the
@param_values
are only passed in temporarily for thedisplay_text
,get_layers
,coerce_parameters
andproduce
callbacks. I guess I would have expected to have exactly one instance of aPCellDeclaration
descendant for each PCell variant. This would've made the tracking of parameter changes a bit more natural in my opinion, as all the parameter values could simply be instance variables.After digging a bit further (and inserting the logging code), I realised that instead, the parameter values are somehow stored (or cached, or both) by KLayout internally and that this is the reason why a
PCellDeclarationHelper
instance only sees the parameter values for the time of a call to the aforementioned callback methods but no longer.As such I can understand what you said by:
Looking at my original code, I don't see any assumptions about this history. When one (or even both) parameters were changed, both size and contacts were updated according to their dependencies, along with their
old_
values. I think the original code meets the requirements of...Except for the additional conditions, your version of the
coerce_parameters_impl
callback should be functionally identical. Further, the logs in my original post suggest there never was a case where I had changed both parameters at the same time (there was always only oneChanging * from * to *
line after eachEntering coerce_parameters_impl
line). As such, the additional conditions should have no effect, unless I have missed something else (which is certainly possible).Therefore I find it even more surprising that using your version of the
coerce_parameters_impl
method in fact did make a difference, the behaviour is now certainly more stable than with my original code. Thank you for that! Given what I said before however, I don't understand why this is the case though... do you have any explanation?In an earlier version of the code, this was actually the case. But I found it cumbersome to have to specify two parameter values when I am actually only interested in one (either the
size
orcontacts
). In a GUI this might not make a huge difference, but certainly when it comes to scripting (which actually is my main interest).Talking about scripting, I also found that
Layout.add_pcell_variant
doesn't callcoerce_parameters
. The pcell_variant method I just published takes care of that and should even allow for conflicting parameters to resolve nicely in that the last one specified wins.The example given illustrates what I intend to do, in that the calling script doesn't have to know anything about "valid" parameter combinations or specify parameters it is not actually interested in (such as a boolean "switch"). Given that method, it should be possible to create a via/contact PCell class similar to the above which has four public parameters: width, height, columns and rows. I am hoping that the
pcell_variant
method can then be called like so...which is just the minimum of required parameters necessary to specify the variant and should coerce the unspecified ones accordingly.
Regards,
Chris
Thinking a bit further about the third paragraph in my last post about having the parameters stored within
PCellDeclaration
instances:That could also simplify (or entirely make obsolete) the
coerce_parameters
callback, along with the need to have anold_
* value for each one of the public parameters for tracking changes.Instead, each parameter could have a
parameter_changed
callback, which would automatically calculate dependent parameters and make sure the specified value was within the allowed range. That could be called within the GUI whenever a new value is entered in one of the input widgets. Which in turn could mean the "Apply" button wouldn't be necessary any more.Admittedly, this would probably require some architectural changes to the way PCells are currently handled in KLayout, but I thought I let you know what I think could be a different way of doing things.
I also noticed that you mentioned here that you are planning to simplify the PCells API... are these changes related to what I just said (or the
pcell_variant
method)?Regards, Chris
Hi Matthias!
I was just by accident looking over my original
coerce_parameters_impl
code again and found a bug which your implementation fixed: when changing thecontacts
, theold_contacts
weren't updated. Using thisinstead of the old
(which was somewhat redundant) fixed the inconsistencies in the GUI. And it makes sense now. It also has the slight advantage over your code (since it is missing the additional conditions) that it never creates inconsistent layout.
Sorry for the confusion and the incorrect "bug report"! Still, I'd be definitely interested in discussing your future plans about the PCell API, especially with regards to simplifying the tracking of changed parameters.
Regards, Chris
Hi Chris,
yes ... that solves the puzzle. By "inconsistent" I meant that old_contacts was not always updated to reflect the previous value.
Regarding "you cannot assume a certain history of events", I mean that you cannot necessarily assume that only contact or size will get changed. It's likely, but within the user interface both parameters are tied to the edit fields. When the edit text changes, Qt sends a signal to my code indicating a change. That behaviour is implementation-dependent and so far it seems to be connect to "loss of focus" or "Enter key pressed". In every case, "coerce_parameters_impl" is called to make the parameters consistent. But maybe Qt changes the behaviour and makes the line edit fields emit the signal for example on initialization, when the value is still empty, then you would suddenly see values that you would not expect. The same could happen if for some reason the focus changes - maybe because a dialog pops up. All these conditions may trigger "coerce_parameters_impl" and maybe your observation of too many calls is connected to that fact. A change in the editable values can also trigger an additional call of "coerce_parameters_impl". The following could basically happen:
If you start using double value, you have to be really careful to take rounding effects into account - otherwise this loop can end in a infinite one because the values may toggle.
Thanks for the "pcell_variant" utility class, by the way. In 0.24 there actually is a method of Layout which basically does what "pcell_variant" does. It'd called "create_cell" and there is a version taking a cell name, a library name and a map of names/values for the PCell parameters. This method is available now, since the Ruby to C++ bridge supports binding of Ruby hashes to C++ maps in 0.24.
However, this implementation calls "coerce_parameter" less often. In fact, it's done just once before the layout is generated. Was there any reason for you to add these calls several times?
Thanks and best regards,
Matthias
Hi Matthias.
Ah, I didn't realise that :-) ...I'm glad we're on the same page now.
I find that surprising. I thought currently the parameter updates through the GUI are caused only by the event of the Apply or Ok buttons? What you described sounds more like what would be on my wish-list for the GUI, namely that the parameter values would update "on the fly" as they're being edited without having to click Apply after each change.
As I did a little bit of GUI programming in the past, I understand that handling focus and update events can sometimes be tricky, but there should be solutions out there. If the
coerce_parameters
method got called after "leaving" one of the input fields (either because the focus has shifted to a different one or because the whole dialog has lost its focus), I think it should be safe to assume that only a single parameter was changed.In my experience the three
coerce_parameters
calls seemed to be triggered by me clicking the Apply button.Then again - since scripting is my main focus at the moment - this is not too high on my priority list.
Indeed. I'm not really sure if there is a good way to avoid the possibility of circular parameter dependencies in general, but the following might be an idea:
PCellDeclaration
instance (e.g. as instance variables such as@contacts
and@size
) to avoid losing state between calls tocoerce_parameters
andproduce
.set_param
method supplied by the PCell developer (as opposed to the one currently generated byPCellDeclarationHelper.param
calls).set_param
method will have to make all parameters consistent, i.e. first ensuring that its "own" parameter is within valid range and then update all dependent parameter values by changing their respective instance variable values. Ifset_size
callsset_contacts
andset_contacts
callsset_size
, this would lead to aSystemStackError: stack level too deep
which would need to be fixed by the PCell developer.contacts
andsize
.size
parameter would call thePCellDeclaration
instance'sset_size
method which would also update the@contacts
value. Subsequently, the GUI would call bothsize
andcontacts
getter methods to update the edit fields again.set_param
methods are called in the order in which they are given by the user's parameter mapping.So now you know my way of thinking about this, it might also help to answer your question:
This is to make sure that subsequent parameter updates (and potential updates of dependent ones) will happen in the correct order and the last ones have priority over the former ones. In a way, I am using the
coerce_parameters_impl
and theold_*
parameters in my example in my original post to emulate the functionality of theset_param
methods I envisioned above.Maybe this is a silly example, but if a user specifies a parameter mapping of
{size: 100, contacts: 3}
, they would probably expect thecontacts
setting to override the (previously set)size
value. As long as they are not specifying two interdependent values, it wouldn't make any difference (other than performance-wise).Having said that, the initial
coerce_parameters
call operating on the initial ("default") values could probably be spared as the PCell developer should probably make sure that the defaults are consistent to begin with.All these ideas aside, I am really glad that we seem to move into the same direction with regards to the PCell scripting interface - your
create_cell
method for 0.24 sounds almost identical to ourpcell_variant
. 0.24 is going to be great!Thank you for your continued efforts to improve KLayout!
Regards, Chris
Hi Chris,
Regarding the GUI interaction you're right - sorry for the confusion. Right now, only Apply will set the parameters. But I was projecting into the future :-)
In general I think the whole handling is a matter of philosophy. I did not have PCell states in mind when I designed the PCell interface, so I'd discourage assuming a stateful approach for the PCell's. The user interface is not the only source of parameter changes - parameters will also happen to be set in scripts, on library updates, when deserializing the PCell instance from a stream and so forth. In all of these cases you cannot control the order the parameters are set and when coerce is called.
To clarify the architecture, it's worth noting that the PCell implementation is not representing an instance. Instead, a PCell instance is simply a special cell carrying some parameters. The PCell implementation class is forming the instance which controls the generation of layout. This scheme follows the "strategy" pattern. So a PCell instance is not a Ruby class and cannot have instance members. For the same reason, there is no parameter setter - any code may modify the instance parameters in any way. But the system will ensure that after every change - either immediate or delayed - "coerce_parameters_impl" and "produce_impl" are called. But at this point, it's unknown what parameter changed and the system is free to compress change events.
"coerce_parameters_impl" is supposed to bring a parameter set into a "consistent" state. I call a parameter set that is returned by this method "consistent".
Given that definition I am able to formulate two rules:
The second rule is important since it enables the transfer of a consistent parameter set into a PCell using any sequence of assignments. Consider a case of three parameters A, B, C and two consistent, but different parameter sets (A1, B1, C1) and (A2, B2, C2). Imagine a PCell is in state (A1, B1, C1) then the following assignment sequences shall lead to the same, consistent state of the PCell (A2, B2, C2):
In other words: a consistent parameter set can be transferred into a PCell safely choosing different paths. A single parameter change will also be possible because of the second rule.
The easiest way to implement the second rule is through a clear distinction between dependent and raw parameters (dependent parameters are computed from the raw ones always). The second-easiest way is through redundancy as CIRCLE does it.
Matthias
Hi Matthias,
This is great news, now I am even more interested in future KLayout versions :-)
Looks like our philosophies are different with regards to the PCell interface :-) ...I guess I was hoping that this would be up for discussion since you hinted here that there might be an architectural change coming up anyway.
I realised that. All I am saying is that I have use cases (and probably will have many more in the future) which would greatly benefit from an architecture which treats each parameter change as a separate event as this would allow for "smarter" PCells such as the contacts I mentioned earlier and at the same time reduce redundancy (the
old_
parameters).It seems to me that using the current architecture allows me to implement a "sequential" parameter change pattern on top of it (as shown in the
pcell_variant
script), albeit with restrictions for when a PCell variant is outside the control scope of a script, e.g. when it is (de-)serialised or manipulated through the GUI provided by the current versions of KLayout (I guess nothing would stop me from implementing my own which only changes parameters sequentially, just as mypcell_variant
code does).As I understand in the current architecture, when a PCell variant is e.g. read from a GDS file, it will find the corresponding
PCellDeclaration
- which is just an array ofRBA::PCellParameterDeclaration
objects - and the array of parameter values. Structurally, this seems to relate to the "Flyweight" pattern.However, since the array of
RBA::PCellParameterDeclaration
objects has to be provided by an instance of aPCellDeclaration
subclass anyway, I don't see the advantage over using the "Chain-of-responsibility" pattern where aPCellDeclaration
instance would be a "processing object" which encapsulates both the parameters' declarations and values. In such an architecture, aPCellDeclaration
instance would be a more "first-class citizen" in terms of responsibility for creating the geometry of a PCell variant.If a
PCellDeclaration
instance was such a "processing object", reading a PCell variant from a GDS file would simply re-create that instance exactly as it was when the corresponding PCell variant was written out.For such an architecture, I can define the following rules:
PCellDeclaration
instance's parameter values are always consistent after a (series of)set_param
method(s) is/are called.PCellDeclaration
instance's parameter values can always be serialised as they are and...PCellDeclaration
instance'sset_param
methods (as the order in which they were defined might be different).PCellDeclaration
instance'sset_param
methods and theparam
methods to read its current state.No redundancy (as in your CIRCLE or my contact PCell) would be required to track parameter changes.
Please be aware that I fully respect your choice of architecture and hope you don't mind me throwing new (and potentially half-baked) ideas at you :-)
Best regards, Chris
Hi again,
I still believe that there is a problem in the Instance Properties dialog of KLayout version 0.24-python-eval when changing interdependent PCell parameter values. I documented the exact steps to reproduce the issue below. I updated the
coerce_parameters_impl
method of theMyPCell
class a bit, the complete code is as follows:These are the steps to reproduce the problem:
MyPCellLib.MyPCell
instance into the "top" cell with the default parameters (Size=100, Contacts=2).Entering coerce_parameters_impl: [100, 100, 2, 2]
Exiting coerce_parameters_impl: [100, 100, 2, 2]
Entering coerce_parameters_impl: [100, 100, 3, 2]
Changing contacts from 2 to 3.
Exiting coerce_parameters_impl: [150, 150, 3, 3]
Entering coerce_parameters_impl: [150, 100, 2, 2]
Changing size from 100 to 150.
Exiting coerce_parameters_impl: [150, 150, 3, 3]
As far as I can see, points 8.1 and 9.1 should give a clue what is happening: While the
coerce_parameters_impl
call at 8. ends with a consistent@param_values
array of[150, 150, 3, 3]
, the call at 9. starts with a different set of values,[150, 100, 2, 2]
. I would have expected the call at 9. to start with[150, 150, 2, 3]
, which means all parameter values remained unchanged except the Contacts value which was changed through the GUI from 3 to 2.Regards, Chris
Hi Chris,
thanks for that heap of information and that nice report.
I'll have a look into that ... given the confusion that I raised, I'll have to check my own code. I'll be back ...
Matthias