fixes to isolate the action of precomp.
Ok, now for maybe a more controversial push.
this whole notion of mutating the "ifo" struct during construction of the noise curves is bad news, but rampant. The "gwinc" call makes a good first start by doing a deepcopy. It seems to me that this deepcopy should be the job of the first function to mutate ifo. In this case precompifo should do the copy.
Furthermore, the beginnings of the "gwinc" function are really additional precomputations, so they have been factored into precompifo.
the F_Hz array is placed in ifo at this point since it already contains dhdl and suspension information that has used F_Hz.
This way future calls can have a simplified API that only needs ifo in the same way that dhdl is given this status.
Really I'm doing this so that optimizer work that wants to work on individual curves within the gwinc call can be made in a more sane fashion.
As a side note to the merge. there should be some other mechanism for this mutation of ifo. It will probably be fixed if we switch to an object-oriented approach to the noise curves. In that, precomputations can be provided by @property
methods and memoized within them... Anyway this merge is an incremental palliative.
Merge request reports
Activity
I have no objection to this change, but a couple of small nit-picks first:
- The import name of the plot module is changed, but I don't see the point of that change or how it relates to the moving of code to the precompIFO function.
- I would like to stick to PEP8 style as much as possible, which includes spaces after '#' in comments and line wrapping at 79 characters.
In general, I think patches should be as simple an focused as possible, and should respect the coding style already in place, or at least conform to PEP8 (I of course realize that much of the existing code does not conform to PEP8, and hopefully it will approach over time).
The change to the plot module name was because it was clashing with the "plot" argument to the gwinc function, so one of them needed to be changed or we need to extricate those commands from the gwinc function as they are redundant with main (as my comment declares.
currently it is a bit of a challenge to prevent any unrelated changes in commits since there are so many to be made.. This merge req for instance is just to preempt needs in a much more involved topic branch (which won't touch core much).
I'm trying to keep pep8 and in general my editor points out the spacing stuff. It seems not to with the TODO, so I'll watch for it.
@lee-mcculler sorry, I missed that bit about the plot namespace collision. i just fixed that in master in a separate patch.
I agree that the entire gwinc() function is a bit redundant. It's there in consideration for the matgwinc interface. I'm certainly fine to toss it out, since I almost certainly will never use it.
added 21 commits
-
1ff152d7...0d086b3c - 20 commits from branch
gwinc:master
- 0399db36 - fixes to isolate the action of precomp.
-
1ff152d7...0d086b3c - 20 commits from branch