Add max_noll_index argument to work with ts_ofc extended to zk28 #37
Conversation
| self.ofc_calc = OFC(OFCData(cam_type.value)) | ||
| self.ofc_calc.ofc_data.zn_selected = np.arange( | ||
| self.ofc_calc.ofc_data.znmin, self.max_noll_index |
There was a problem hiding this comment.
I think we assume we start at Z4 everywhere in ts_imsim. Should we set ofc_calc.ofc_data.znmin = 3 somewhere so that OFC expects to receive what ts_imsim is set up to pass in case the default changes for whatever reason in ts_ofc?
| parser.add_argument( | ||
| "--max_noll_index", | ||
| type=int, | ||
| default=22, |
There was a problem hiding this comment.
Do we want to switch default to 28? Are we planning on using 28 for commissioning tests?
| @@ -65,6 +66,9 @@ def __init__(self) -> None: | |||
| # imSim Component | |||
| self.imsim_cmpt = None | |||
|
|
|||
| # Maximum Noll index | |||
| self.max_noll_index = 22 | |||
There was a problem hiding this comment.
Does this have to be a class variable? Can we pass it through like all other options except boresight variables?
There was a problem hiding this comment.
Well, the max_noll_index is used in different functions and I thought it was cleaner for it to be a class variable instead of having to pass it down as an attribute through multiple nested functions
There was a problem hiding this comment.
Ok. But let's set it to None here like the rest so that the default value will come from the command line arguments. How's that sound?
| @@ -50,7 +50,7 @@ class ImsimCmpt: | |||
| generate full imsim configuration files. | |||
| """ | |||
|
|
|||
| def __init__(self) -> None: | |||
| def __init__(self, num_of_zk: int) -> None: | |||
There was a problem hiding this comment.
Can you add the parameter to the class docstring above?
gmegh
force-pushed
the
tickets/DM-43390
branch
2 times, most recently
from
c077b66
to
dfed356
Compare
No description provided.