Skip to content
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

Missing implementation for the density_mx evotype's "identity plus error gen" OpRep #513

Open
rileyjmurray opened this issue Dec 10, 2024 · 0 comments

Comments

@rileyjmurray
Copy link
Contributor

PR #452 uncovered this commented out block of code.

#TODO: can add this after creating OpCRep_IdentityPlusErrorgen if it seems useful
#cdef class OpRepIdentityPlusErrorgen(OpRep):
# cdef public object errorgen_rep
#
# def __init__(self, errorgen_rep):
# self.errorgen_rep = errorgen_rep
# assert(self.c_rep == NULL)
# self.c_rep = new OpCRep_IdentityPlusErrorgen((<OpRep?>errorgen_rep).c_rep)
# self.state_space = errorgen_rep.state_space
#
# def __reduce__(self):
# return (OpRepIdentityPlusErrorgen, (self.errorgen_rep,))
#
# #Needed?
# #def errgenrep_has_changed(self, onenorm_upperbound):
# # pass

During review of that PR, Corey asked

Which rep type currently covers the identity plus error generator object? Is this already covered, or are we deficient in this regard in which case should this be re-enabled?

Corey subsequently looked into this, and came back with this assessment.

This should be resuscitated. If I understand the code correctly then this will result in an error when using this modelmember if the representation type is '1+L'. There is an implementation of this representation in densitymx_slow, so an error would get hit when using the cython module (though that is the default when present).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant