search

oda-hub / dispatcher-plugin-integral

Other
1 stars 1 forks source link

inconsistency in ISGRI and JEM-X product show #40

Closed ferrigno closed 3 years ago

ferrigno commented 3 years ago

from oda_api.api import DispatcherAPI disp=DispatcherAPI(url='http://dispatcher.staging.internal.odahub.io', instrument='mock') par_dict={\ "src_name": "4U 1700-377",\ "RA": "270.80",\ "DEC": "-29.80",\ "T1": "2021-05-01",\ "T2": "2021-05-05",\ "T_format": "isot",\ "instrument": "jemx",\ "osa_version": "OSA11.0",\ "radius": "4",\ "max_pointings": "50",\ "integral_data_rights": "all-private",\ "jemx_num": "1",\ "E1_keV": "3",\ "E2_keV": "20",\ "product_type": "Real",\ "detection_threshold": "5",\ "product": "jemx_lc",\ "time_bin": "4",\ "time_bin_format": "sec",\ "catalog_selected_objects": "1,2,3", "selected_catalog" :'{"cat_frame": "fk5", "cat_coord_units": "deg", "cat_column_list": [[0, 1, 2], ["GX 5-1", "MAXI SRC", "H 1820-303"], [96.1907958984375, 74.80066680908203, 66.31670379638672], [270.2771301269531, 270.7560729980469, 275.914794921875], [-25.088342666625977, -29.84027099609375, -30.366628646850586], [0, 1, 0], [0.05000000074505806, 0.05000000074505806, 0.05000000074505806]], "cat_column_names": ["meta_ID", "src_names", "significance", "ra", "dec", "FLAG", "ERR_RAD"], "cat_column_descr": [["meta_ID", "<i8"], ["src_names", "<U10"], ["significance", "<f8"], ["ra", "<f8"], ["dec", "<f8"], ["FLAG", "<i8"], ["ERR_RAD", "<f8"]], "cat_lat_name": "dec", "cat_lon_name": "ra"}', "token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJlbWFpbCI6IkNhcmxvLkZlcnJpZ25vQHVuaWdlLmNoIiwibmFtZSI6ImNmZXJyaWdubyIsInJvbGVzIjoiYXV0aGVudGljYXRlZCB1c2VyLCBhZG1pbmlzdHJhdG9yLCBjb250ZW50IG1hbmFnZXIsIGdlbmVyYWwsIGludGVncmFsLXByaXZhdGUtcWxhLCBtYWdpYywgdW5pZ2UtaHBjLWZ1bGwsIHB1YmxpYy1wb29sLWhwYywgYW50YXJlcywgc2RzcyIsImV4cCI6MTYyMzEzNDg5M30.UVaNwQBGdcUly3bsQDH17X0Wa9fPz8nTWyphHP6yoBY" }

data_collection_lc.show()

ID=0 prod_name=jemx_lc_0_H1820303 meta_data: {'src_name': 'H 1820 303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

The "-" in the source name of "H 1820-303" disappears in the "show()" method.

ferrigno commented 3 years ago

light_curve = disp.get_product(instrument="isgri", product="isgri_lc", product_type="Real", osa_version='OSA11.0', RA=275.09142677, DEC=7.18535523, radius = 8, T1=58193.455, T2=58246.892, E1_keV=30, E2_keV=80, T_format= 'mjd', max_pointings=50, time_bin=1000, #time bin in seconds token=token, selected_catalog=api_cat_str)

light_curve.show()

ID=0 prod_name=isgri_lc_0_MAXIJ1820+070 meta_data: {'src_name': 'MAXI J1820+070', 'time_bin': 0.0115740651235683, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=isgri_lc_1_GRS1915+105 meta_data: {'src_name': 'GRS 1915+105', 'time_bin': 0.0115740439999475, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

We get the lightcurve that we care about (note that '+' is replaced by 'p' and '-' by 'm')

lc_maxi=light_curve.isgri_lc_0_MAXIJ1820p070

ferrigno commented 3 years ago

It would be good that "show()" indicates the name of the attribute as it will be used.

burnout87 commented 3 years ago

ID=0 prod_name=jemx_lc_0_H1820303 meta_data: {'src_name': 'H 1820 303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

The "-" in the source name of "H 1820-303" disappears in the "show()" method.

Would you expect here to have:

ID=0 prod_name=jemx_lc_0_H1820m303 meta_data: {'src_name': 'H 1820 303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'} ?

Whereas for ISGRI, the output of show() is as expected?

ferrigno commented 3 years ago

They are NOT as expected for both ISGRI and JEM-X

They should be something like (substitute isgri for jemx in case)

prod_name=jemx_lc_0_H1820m303 meta_data: {'src_name': 'H 1820-303' prod_name=jemx_lc_0_H1820p303 meta_data: {'src_name': 'H 1820+303' prod_name=jemx_lc_0_MAXISRC meta_data: {'src_name': 'MAXI SRC'

burnout87 commented 3 years ago

By doing some test and digging I noticed that for jemx_lc the following src_name are returned:

As far as I understood, this is not how it should return. It's also the reason why show() (as well as as_list()) do not behave as expected for jemx. Can you confirm it? I just used the tests posted here above

There is also another problem with the function, PR will follow soon.

burnout87 commented 3 years ago

I just re-run the previous test for JEM-X lc, and this is the output of show():

ID=0 prod_name=jemx_lc_0_H1820m303  meta_data: {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC  meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

For completeness, I also gave a try to as_list():

[{'ID': 0, 'prod_name': 'jemx_lc_0_H1820m303', 'meta_data:': {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}}, {'ID': 1, 'prod_name': 'jemx_lc_1_MAXISRC', 'meta_data:': {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'E
volodymyrss commented 3 years ago

so that's what you wanted, yes? Seems ok.

volodymyrss commented 3 years ago

I just re-run the previous test for JEM-X lc, and this is the output of show():

ID=0 prod_name=jemx_lc_0_H1820m303  meta_data: {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC  meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

For completeness, I also gave a try to as_list():

[{'ID': 0, 'prod_name': 'jemx_lc_0_H1820m303', 'meta_data:': {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}}, {'ID': 1, 'prod_name': 'jemx_lc_1_MAXISRC', 'meta_data:': {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'E

oh, could we add a code snipplet that verifies this in there https://github.com/oda-hub/oda_test_kit

it's used for integration tests.

burnout87 commented 3 years ago

oh, could we add a code snipplet that verifies this in there https://github.com/oda-hub/oda_test_kit

Ok, can I use this test (the one here discussed)? What is the main purpose of those tests?

burnout87 commented 3 years ago

oh, could we add a code snipplet that verifies this in there https://github.com/oda-hub/oda_test_kit

Ok, can I use this test (the one here discussed)? What is the main purpose of those tests?

PS I also need to be added to the repo

volodymyrss commented 3 years ago

oh, could we add a code snipplet that verifies this in there https://github.com/oda-hub/oda_test_kit

Ok, can I use this test (the one here discussed)? What is the main purpose of those tests?

here:

it's used for integration tests.

i.e. it's used for integration tests. I.e. the tests on the entire platform, end-to-end. They are also run in production regularly.

Please make a PR.

burnout87 commented 3 years ago

I still need access rights

volodymyrss commented 3 years ago

I still need access rights

oh, sorry, but I do not understand - it is a public repository, yes? Is it possible to see it? To fork it? It should be!

burnout87 commented 3 years ago

PR open

volodymyrss commented 3 years ago

thanks!

volodymyrss commented 3 years ago

I think this is done, yes? Let's see that @ferrigno verifies.

volodymyrss commented 3 years ago

strange, I ran it on a different data set (https://github.com/oda-hub/oda_test_kit/blob/master/test_lc.py#L53) and git this:

ID=0 prod_name=jemx_lc_0_H1820-303  meta_data: {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC  meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

len(l_output): 
 2
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/savchenk/oda_test_kit/test_lc.py", line 53, in test_jemx_lc_source_name_formatting
    assert l_output[0]['prod_name'] == 'jemx_lc_0_H1820m303'
AssertionError

this is the plugin deployed https://github.com/oda-hub/dispatcher-plugin-integral/tree/5f7df47010c9652a718ec054745139fa5c3cac07

volodymyrss commented 3 years ago

could you confirm @burnout87 ? we could try few different variants

burnout87 commented 3 years ago

I just re-run it, and for me it still works. I run the very same test

volodymyrss commented 3 years ago

I just re-run it, and for me it still works. I run the very same test

the very same test you ran before or the same I updated?

burnout87 commented 3 years ago

Actually yours.

And this is the output:

ID=0 prod_name=jemx_lc_0_H1820m303  meta_data: {'src_name': 'H 1820-303', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ID=1 prod_name=jemx_lc_1_MAXISRC  meta_data: {'src_name': 'MAXI SRC', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}
burnout87 commented 3 years ago

Are you running the last version of the oda_api ?

volodymyrss commented 3 years ago

yeah, I realized that was the problem. I made the requirement explicit: https://github.com/oda-hub/oda_test_kit/blob/master/test_lc.py#L20

burnout87 commented 3 years ago

I think we can close this now ?

volodymyrss commented 3 years ago

maybe let's let @ferrigno review.

By the way, I made this:

https://github.com/oda-hub/doc-handbook#dealing-with-issues-emerging-in-review-and-operations

I should have made it through PR so we can all review and comment, sorry. Please feel free to suggest changes. Also let's discuss today 15h.

ferrigno commented 3 years ago

It does not work currently

ID=1 prod_name=isgri_lc_1_H0614+091 meta_data: {'src_name': 'H 0614+091', 'time_bin': 0.000578675871775676, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

should be

ID=1 prod_name=isgri_lc_1_H0614p091 meta_data: {'src_name': 'H 0614+091', 'time_bin': 0.000578675871775676, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

ferrigno commented 3 years ago

ID=0 prod_name=jemx_lc_0_1A1742-294 meta_data: {'src_name': '1A 1742-294', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'} should be ID=0 prod_name=jemx_lc_0_1A1742m294 meta_data: {'src_name': '1A 1742-294', 'time_bin': 4.62963e-05, 'time': 'TIME', 'rate': 'RATE', 'rate_err': 'ERROR'}

volodymyrss commented 3 years ago

but did you update oda_api?

ferrigno commented 3 years ago

but did you update oda_api?

No, now it is correct with v 1.1.15