Locals#266
Conversation
Allow `units` to not be specified + unit test
romanc
left a comment
There was a problem hiding this comment.
Have you considered making a temporary quantity, i.e.
class Temporary(Quantity):
def __init__(...):
super().__init__(...)
# Note: I wouldn't expose `_transient` in the constructor
# of `Quantity` because it will only raise questions.
self._transient = TrueThat would still allow to group all temporaries in a State object, e.g.
@dataclass
class MyTemporaries(State):
tmp_a: Temporary = dataclass.field( ... )
tmp_b: Temporary = dataclass.field( ... )It would open the possibility of State objects with Quantity and Temporary members, which we can't do with the current proposal. That might or might not be a good idea. I'm not familiar enough with what atmospheric scientist expect from the "State" concept. My CS-background finds it weird that I have to group my temps and non-temps separate states.
|
I share Roman's perspective. If I understand the goal right, we want to have a system that replaces the class-based temporary-initialization work done for example here: |
|
Post NASA team discussion:
|
twicki
left a comment
There was a problem hiding this comment.
This looks really clean, I like this approach.
There is not really a need for a separate (inheriting) class Locals because we can create a frontend-api that makes it very obvious, right? The "problem" I see with an optional kwarg is that people might drop it. We can assume our users are clever enough to not do that, right?
|
PR updated with code sample & explanation of the issue/remedy |
|
All right the basics are in. We have a We have a
|
Move all unit test into a `test_ndsl_runtime`
romanc
left a comment
There was a problem hiding this comment.
This looks nice to me. I'm glad we did a couple rounds of prototyping.
Locals
AS: All of the above is geared to, and only to,
orch:dace:X- but the system will be deployed at NDSL level in a backend-blind way.A limitation to our optimization pipeline is the inability to differentiate "global" fields from "transient". Take the following code:
In this configuration the field
self._local_fieldis technically "temporary" toPhysicsParamcode and should therefore betransientin DaCe SDFG lingo. Multiple problems arise:Quantityastransient. Should we? This is both an highly technical feature, but it has an explainable usage.orch:Xbackends will have memory irrelevant inself._local_fieldsince DaCe would take over it. All other backends would still have the last value usedTo address those, we introduce a concept of
Localwhich is a field that is forbidden to be used outside of the module it was defined in. In the previous exampleself._local_fieldwould become aLocal. ThisLocalwould pass thetransientflag to thedace.Datadescriptor and open the way for futher optimization.From this, we need to implement a few systems:
Localsoutside of the module that allocated it. E.g.:Quantityallocation within "NDSL" code to warn about potential under-optimized use.Below is a proposal - which has been tested for orchestration:
Remained to be solved:
LocalFactory? Method onNDSLRuntimetaking aQuantityFactory? ... ?)PS: This also introduces a
NDSLRuntimebase class, which can be used for a betterorchestration,debugand overall gives consistency and structure to NDSL code