-
Notifications
You must be signed in to change notification settings - Fork 4
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
matrices not updated in bilateral model #68
Comments
observation_matrix
not updated in bilateral model
I noticed the same issue occurs for the transition matrix. Doesn't your solution create an infinite loop? Because when changing the ipsilateral modality, it triggers the contralateral callbacks, which, in turn, should trigger the ipsilateral callbacks. That's why I defined these Currently, I am thinking of an implementation similar to what I did with the |
Interestingly, it does not create an infinite loop Seemingly, the change of the contralateral modalities does not trigger a second callback since only the ipsilateral should be triggering the callback. Which probably means, that when setting contralateral modalities, no callbacks are initialized and the ipsilateral side is not synced. I.e. we only have a one way syncing. Generally I would welcome a syncing approach that is global as you implemented before. |
Alright, then I'll work on that now: A two-way sync for the modalities via the callbacks and globally cached functions for the transition and observation matrices. |
The graph `Representation` class now has a method to compute a hash that solely depends on the parametrization of the edges in the class. This may be used to skip recomputing the transition matrix when that hash dis not change. Related: #68
This cache decorator takes the first argument (arg0) that is passed to the decorated function and checks the cache based on its (hash) value. The decorated function then gets executed with the remaining arguments. Related: #68
Like the transition matrix, the observation and diagnose matrix are now computed by a globally cached function. Related: #68
The new way of globally caching the matrices required some refactoring and simplifying. Fixes: #68
Ok, this was a bit trickier than I thought 😅 Because the
If either changes, it should be recomputed. So, I had to compute a hash for all three quantities (ideally somewhat fast) and combine them into one hash to make sure you always get the correct diagnose matrix. The changes are now in the So, it'd be great if you could double check that you can still reproduce your results. Thanks! |
I'll go through it on Monday to double check that everything is fine. Thanks for the changes! |
I tested the trinary and the binary model with bilateral sampling and risk calculations. The results were good --> seems like everything works as expected. The computation time is only minimally increased (might also just be some CPU fluctuations^^). Looks like a perfect fix! |
While debugging my code I stumbled over an interesting problem which only arises when changing the modalities of a bilateral model.
Updating the
modalities
in any model should update theobservation_matrix
. This also works for the ipsilateral side, but not for the contralateral side. It seems as if callback for the contralateral side is not successful. I am not 100% sure whether this is due to the synchronization or if there is another problem.I am working on a fix, but I wanted to notify you since you probably can detect the problem faster and produce a better fix.
Edit: my fast fix was quite simple. Since the problem is that the contralateral side does not trigger callbacks anymore, I am applying a different syncing function for the modalities:
Thus we also need to change the way modalities are synced in the
init_synchronization
function:I am sure that you can come up with a more elegant solution, as I am struggling a bit to get a good overview of all the callbacks and syncing functions.
The text was updated successfully, but these errors were encountered: