Comments (11)
@anetasie I can reproduce this. A simple workaround is to call ungroup()
before grouping again.
I guess it makes sense that data should be ungrouped before it can be regrouped, it this correct?
If it is, I need more information regarding the fix: the simplest fix I can think of involves refusing to apply a new group function on a dataset that has already been grouped, and showing the user a WARNING.
An alternative involves automatically calling ungroup()
if a group function is called and the dataset has already been grouped.
I can also see possible combinations of these two possibilities (e.g. refuse to group a grouped dataset, but accept a keyword argument to force the application of ungroup()
).
What do you think?
from sherpa.
I don't think we should refuse to apply the new grouping. Sherpa does not have support for hierarchical grouping, so the data needs to ungroup before applying a new group. I would say that this can be done automatically by calling ungroup()
. Although, I see the reason for just refusing to apply a new grouping and exit with error. I'll discuss with SDS and come back with the final answer.
from sherpa.
Sherpa does not have support for hierarchical grouping,
Are there any plans to support hierarchical grouping in the foreseeable future?
from sherpa.
We did not plan on this and I don't think this is something with the high priority for the future.
from sherpa.
Kenny provided a good example for the automatically applying ungroup
before grouping.
If you're trying to "optimize" the grouping you're likely doing something like
group_counts(10)
plot_data()
group_counts(15)
plot_data()
group_counts(12)
plot_data()
so this would require extra ungroup
with each group()
and having sherpa automatically applying ungroup
makes sense.
from sherpa.
Also we will follow the XSPEC convention if we ugroup()
automatically.
from sherpa.
Sounds good. Thanks for the info.
from sherpa.
Trying to come up with a minimal set of steps to reproduce the issue (and thus a test case), I realized that the issue might have more to do with filtering rather than grouping, i.e.:
In [1]: from sherpa.astro.ui import *
In [2]: load_data("sherpa-test-data/sherpatest/ciao4.3/pha_intro/3c273.pi")
WARNING: [...]
In [3]: notice(0.3,2)
In [4]: group_counts(30)
In [5]: group_counts(15)
---------------------------------------------------------------------------
IndexError Traceback (most recent call last)
results in an error. However, the following code does not:
In [1]: from sherpa.astro.ui import *
In [2]: load_data("sherpa-test-data/sherpatest/ciao4.3/pha_intro/3c273.pi")
WARNING: [...]
In [3]: group_counts(30)
In [4]: group_counts(15)
In [5]:
The only difference is the call to notice(0.3,2)
from sherpa.
Yes, @DougBurke suggested that there are issues with filtering and grouping, since Sherpa always starts grouping from the first channel in the data regardless of the applied filter, while when the data is filtered the user expectation is that the grouping would be only applied to the data within the valid filter.
So the logic here may need to be reviewed in more details.
from sherpa.
@anetasie The fix to this issue might actually be simpler than I expected. I am going to issue a PR as soon as I am done with the PEP8 changes. However, we should check whether we have enough testing coverage for the grouping code, especially if we plan to make significant changes.
So far, the only test exercising the group_counts
method is this:
https://github.com/sherpa/sherpa-test-data/blob/master/sherpatest/ciao4.3/setfullmodel/fit.py
We should make sure that is enough. If it is not, then we should add more tests. It could be a good idea to add some unit tests, as in the above thread the grouping results are only checked against the results of the fit (or they are not checked at all). So, even if the tests may cover the grouping code, it is not clear whether meaningful tests are actually executed.
In any case I created a new test as part of the patch for this bug.
In lieu of a proper PR, here is the fix:
https://github.com/sherpa/sherpa/compare/bug-%2338-grouping-twice
from sherpa.
Just to note that this is a slightly tricky bug to tickle, since it doesn't happen if (using the example from #38 (comment) ) I don't see the error if I use ui.notice(0.5, 8)
rather than ui.notice(0.3, 2)
.
from sherpa.
Related Issues (20)
- Can we improve error messages in UI by listing the dataset ids that cause error? HOT 1
- Move ignore/notice and binning from DataPHA to Data1DInt (or even higher in the hierachy) HOT 2
- can not use spawn or forkserver methods for multiprocessing HOT 1
- Should we replace NoNewAttributesAfterInit with __slots__ HOT 1
- Inconsistent naming for axis between 2D models and 2D data HOT 1
- 2D data looks crazy - is it mixing up x0/x1 in some cases? HOT 5
- plot class hierarchy issues
- build issues on Mac OS with arm64 HOT 3
- How to load a pha object into an UI session HOT 3
- Looking into EstNewMin
- Switch numpy.matrix to numpy.ndarray
- issues with xspec HOT 6
- note about occasional ocdecov failures
- Test fails: signal only works in main thread of the main interpreter HOT 8
- x-ray polarization stats HOT 1
- arch build has switched to using python 3.12 so the weekly build now fails
- sherpa_test requires matplotlib HOT 5
- Pytest warning: PytestUnknownMarkWarning: Unknown pytest.mark.xdist_group` HOT 2
- XSPEC tests can crash on macOS with XSPEC 12.14.0 HOT 1
- add an optimizer/stat to the UI layer
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sherpa.