Observables #123
Merged
Observables #123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## py_dev_hsp #123 +/- ##
=============================================
Coverage ? 69.18%
=============================================
Files ? 34
Lines ? 1233
Branches ? 164
=============================================
Hits ? 853
Misses ? 380
Partials ? 0
Continue to review full report at Codecov.
|
|
This looks great, and I like the renaming to EventNumber. What is the GooFit index? I believe I completely bypassed this in the indexing commit. |
|
It's the index into the goofit event. 0-n, where n is the observable or the variable max number. It's kind of hacky and keeps growing in the unit tests. |
|
It's not a hack, it's an inspired kludge. 😁 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: this is a PR into the Python branch.
This splits Variables into two categories; Variables and Observables. You used to select between them with a magic number of parameters; now they are separate classes. This facilitated a cleanup of what an Indexable is, and specialized each of the class appropriately. For example, only an Observable has a number of bins, only a Variable has a FitterIndex. The API throughout GooFit has been adapted to use Observable as needed; you can't mix the two up anymore. Internally, GooFit always has treated them separately.
One related change; the GooFit index now gets assigned the first time a Variable is used in a PDF, and statically increments. The old Variable map has been axed.
CountingVariable has been renamed to EventNumber, since that's what it is, and because it's an Observable, not a Variable.
This also fixes the copyable Variables bug introduced in the last few commits to the python branch. Variables are still compared via address instead of internal address; that will be fixed soon. The final stage of that change would be removing Variable pointers everywhere, and instead using copies directly. This would be a huge help to both PyGooFit and AmpGen2GooFit, and to average users.