igrins / plp

IGRINS pipeline package v3
18 stars 16 forks source link

qlook branch does not use correct standard #30

Closed ericasaw closed 5 months ago

ericasaw commented 6 months ago

Hi @leejjoon this is something you might want to be aware of in your reductions!

In the old pipeline GROUP2 in the recipe file corresponds to the filenumber of the standard that you want to use for the reduction. In the new pipeline this filenumber is not always followed.

We were comparing spec_a0v.fits files from the old and new pipeline to observe improvements in the flexure correction but noticed sometimes there was a large change in flux between the two pipelines for seemingly random groupings of observations throughout the night. It turns out in these cases that new pipeline is not listening to the GROUP2 filenumber and instead looks to be picking a standard filenumber based on proximity in the recipe log (?maybe?).

Here is an example from 20161013 if you want to verify yourself:

HR  1237 V 6.3, STD, 276, 1, 30.000000, A0V_AB, 276 277 278 279 280 281 282 283, A B B A A B B A
HBC388, TAR, 284, 276, 60.000000, STELLAR_AB, 284 285 286 287 288 289, A B B A A B
IPTau, TAR, 290, 276, 60.000000, STELLAR_AB, 290 291 292 293 294 295, A B B A A B
V819Tau, TAR, 296, 276, 60.000000, STELLAR_AB, 296 297 298 299 300 301, A B B A A B
LkCa14, TAR, 302, 276, 60.000000, STELLAR_AB, 302 303 304 305 306 307, A B B A A B
V836Tau, TAR, 308, 276, 60.000000, STELLAR_AB, 308 309 310 311 312 313, A B B A A B
FTTau, TAR, 314, 276, 60.000000, STELLAR_AB, 314 315 316 317 318 319, A B B A A B
167-317, TAR, 320, 328, 180.000000, STELLAR_AB, 320 321 322 323 324 325 326 327, A B B A A B B A
HD  65158 V 7.2, STD, 328, 1, 30.000000, A0V_AB, 328 329 330 331, A B B A

All of the objects before filenumber 320 should be using the same standard (276) but I find that with the new pipeline file numbers 302, 308, and 314 actually use the standard 328. Here are some plots showing the standard comparisons

Between standards used in 314 and 320--they should be different but are the same: image

Between the standards used in 314 and 296--they should be the same but are different: image

ericasaw commented 6 months ago

Okay I think I've tracked down what went on here nominally the a0v_obid.py in both the old and new pipeline look the same the difference the difference is when the a0v variable is set in both pipelines.

in the old pipeline a0v == "GROUP2" before it reaches the filter_a0v meaning when a0v reaches filter_a0v in recipe_base.py for the old pipeline (a0v_obsid.py in the new pipeline) it enters the first if statement:

def filter_a0v(a0v, a0v_obsid, group2):
    # print master_obsid, a0v_obsid
    if a0v is not None:
        if a0v_obsid is not None:
            raise ValueError("a0v-obsid option is not allowed "
                             "if a0v opption is used")
        elif str(a0v).upper() == "GROUP2":
            a0v = group2
    else:
        if a0v_obsid is not None:
            a0v = a0v_obsid
        else:
            # a0v, a0v_obsid is all None. Keep it as None
            pass

    return a0v

in the new pipeline the a0v variable is not set before it is passed into filter_a0v meaning GROUP2 is never used even when it is set. Instead filter_a0v always returns None so the catch for errors in recipe_divide_a0v.py:

if a0v_obsid is None:
      a0v_obsid_ = obsset.query_resource_basename("a0v")
      a0v_obsid = obsset.rs.parse_basename(a0v_obsid_)

So even when GROUP2 is set in the recipe file the new pipeline will always just seek out the nearest A0V in the recipe to use instead.

kfkaplan commented 5 months ago

With the release of v3.0.0, I think this issue has been fixed and the qlook branch is obsolete. I am going to close this issue. @ericasaw if this is still a problem reopen the issue and let me know.