-
Notifications
You must be signed in to change notification settings - Fork 32
Submodel machinery and improvements
- Right now ty files still instantiate everything (expensive) without offering a chance for anything else to look at the model parameters cheaply (e.g. Lancet checking parameters).
- Proposal: the ty file should not call the model object to instantiate the model. Instead, it should 'install' the model on
topo.sim.model
. - Proposal: Make topo.sim callable, use
topo.sim()
to instantiate the model (obviously an error will be generated if the model is not specified). - Proposal: Calling
topo.sim.run
will automatically instantiate the model if it has not done so already. - Proposal: For compatibility with old ty files, there needs to be checks if the model has been instantiated directly by some other means.
- Hopefully this system would allow Lancet / run_batch to look at the model object before any instantiation happens.
-
Sheet
objects should also have properties. -
Models need a sensible
__repr__
-
Warn if matchcondition defined (and not None) and no projections were created (for that condition)
-
Tobias made the following comment in issue #573 :
The current implementation does not work with SheetSpec properties which have non-string values, as
name+=prop
fails (name is a string). This is currently the case for spatial frequency sheets.There are two ways to fix this:
- Properties must always have string values, which would need to be documented. The drawback with this fix is that things like
channel=properties['SF']
if 'SF' in properties else 1 would become more slightly more complicated, i.e.channel=int(properties['SF'][2:]) if 'SF' in properties else 1
. However, it would have the advantage that we could stick to the current naming convention. - The simplest solution would be changing
name+=prop
toname+=str(prop)
. However, then SF sheets would be called LGNOn1, LGNOn2 and so forth. As long as there is only SF, this is fine. However additional "integer-"properties would make it harder to distinguish which sheet is meant, at least from the sheet name. Internally, everything would be still nice because the properties can always be accessed.
- Properties must always have string values, which would need to be documented. The drawback with this fix is that things like
-
Jim made this comment about the attribute access being over-specified in #573 :
Right. I wouldn't want to enforce globally unique projection names, but note that incoming projections are guaranteed to have unique names for a given target sheet, so in principle
gcal.projections.V1.LGNOn.LGNOnAfferent
could be done asgcal.projections.V1.LGNOnAfferent
. In practice there may be some other reason to have LGNOn explicitly, which is fine if so.
- Functionality to inspect the structure of
SheetSpec
andProjectionSpec
objects to check the network has one fully connected component. - Render a
Model
instance as a model diagram (functionality currently only offered by the Model Editor)