-
Notifications
You must be signed in to change notification settings - Fork 9
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
Deprecated ContextWindowDataset.observables attribute? #10
Comments
Hi, @Danfoa! Thanks for opening this issue. Indeed, the The idea of the API is that upon creating an instance of As of now, the way to create an ctxs = ContextWindowDataset(data)
ctxs.observables = {
'obs_1': data_obs_1,
'obs_2': data_obs_2
} which as you say is not a good practice. To address this issue, we can start by modifying Line 9 in 3568b40
by adding an observables property with getter and setter methods, where the setter methods check that the shape of the first two dimensions of ctxs.data and ctxs.observables['obs_1'] coincide. Recall how the first two dimensions of ctxs.data have shape number of contexts, context length.
It is reasonable to initialize the
I am assigning this issue to myself and @Danfoa. Let's create a branch out of |
Just to verify, if i have a series and want to compute I need to have in the observable dictionary an array containing the What would be the behaviour of the And finally, should we allow for functionals in the |
Establishing a shared nomenclature for these concepts is crucial from the outset, especially since the term "observables" remains somewhat ambiguous to me within our context. In the Kooplearn framework, to ensure clarity both among ourselves and for our users, it's essential to have well-defined terminology and documentation differentiating between:
While the terms I've used are suggestions, our aim should be to clearly delineate these categories in our documentation and nomenclature for transparency and ease of understanding. |
My definitions, which I try to use throughout StatesA state of the dynamical system/stochastic process is usually denoted @Danfoa, I know that according to the previous description, states are not defined uniquely. For example, any bijective transformation of a state (which in turn is an observable, see below) is again another state. As the algorithms in In short: a state in ObservablesObservables are arbitrary functions of states Given these definitions, we can further distinguish between
To answer @GregoirePacreau's question:
kooplearn/kooplearn/models/kernel.py Line 395 in 3568b40
To take the ctxs = TensorContextWindow( ... )
ctxs.observables = {
'f': data_obs_f
}
modes, eigs = model.modes(ctxs)
modes_f = modes['f']
#i-th mode of f
modes_f[i] |
Something to add to this PR I just noticed: Handling observables is a bit awkward, as calling This should be fixed: observables should be defined on the train data from the get-go, and the dict-like should contain |
Have a look at this DataClass, which I use to store observations (including state) from a given Markov dynamical system. The key idea is to keep observables separated and named, for instance:
If you want all/some stacked observables you get a view of the data. In general this added structure of knowing which parts of the state are vectors, scalars, etc is needed to handle the system's symmetries. Adding this structure to a That DataClass has become quite helpful for me, maybe there is something there we can adapt to kooplean. In general to solve the problem of the 5 inheritance classes, I propose to introduce a single ContextWindowLike class which is either a wrapper or direct inherited from Tensor/ndarray. Similar in spirit as GeometricTensor, which wraps a tensor and offers some additional structural attributes and methods. From what I can see, the idea behind introducing the
In practice, when using torch, the Dont see the need for the 5 levels of abstraction, which in practice are/will become problematic. |
I noticed that the
BaseModel
class makes reference to anobservables
attribute from theContextWindowDataset
class, which seems to be undefined and undocumented within that class. For example, this issue can be observed here:kooplearn/kooplearn/abc.py
Lines 21 to 40 in fe49ab7
The
data.observables
attribute is neither defined nor documented in theContextWindowDataset
, which leads to confusion. Similarly, the purpose of thepredict_observables
attribute in the base model’spredict
method is unclear.I suggest that this attribute should either be clearly documented and defined within the
ContextWindowDataset
class or removed from the function call and documentation to avoid confusion. Generally, dynamically adding attributes to class instances without prior definition or documentation is not a recommended practice.Additionally, it would be beneficial to establish a clear vocabulary for the types of observable functions. Currently, it's not clear whether "observables" refer to state observables, which are scalar/vector-valued functions analytically computed/measured from the dynamical system, or to state latent observables/features, which are scalar/vector-valued functions we learn or define based on state observables.
Does kooplearn have an established nomenclature for distinguishing these types of observables? @pietronvll @vladi-iit @g-turri Should we make this distinction?
The text was updated successfully, but these errors were encountered: