samjtro / go-algotrader

pure go schwab algotrading
Apache License 2.0
2 stars 1 forks source link

MACD not being added to DataSlice #2

Closed samjtro closed 2 years ago

samjtro commented 2 years ago

When I return the MACD() function, for some reason none of the information I've added to []DATA for the MACD parameter for each interval is missing, just listed as the empty val 0. Unsure of what I'm doing wrong as it's basically the same exact code as the earlier functions? Potentially it has something to do with the way I am using EMA and iterating through the resulting []DATA structs. Will do some more testing.

samjtro commented 2 years ago

The same issue is now occuring with Chaikin(). This is incredibly frustrating.

samjtro commented 2 years ago

This issue has been completed via: eade6400fcd8edc287ca634409a3984dffe8d955

However, I will note this was more resolved with me using d[i].MACD instead of _,x := range x { x.MACD }. One references a variable set in iteration whereas the other adds the .MACD indicator to the DataSlice.

samjtro commented 2 years ago

This is incredibly frustrating; now, VWAP & MACD are both zero values in each from of the DataSlice. Not sure what is causing this to happen. I am going to need to do some further analysis into what the function is doing during the call that causes this error to arise, I'm sure it's something simple I've missed but still need to find it.

The original issue was fixed when I changed from using x for i,x := range d to using d[i], therefore referencing the DataSlice rather than the element ranged over.

However, I have used d[i] and now it is still not working.

tanryberdi commented 2 years ago

@samjtro I need your help to create app on https://developer.tdameritrade.com/, I registered as user, but i could not create app, actually idk what was the name for app ))

samjtro commented 2 years ago

@samjtro I need your help to create app on https://developer.tdameritrade.com/, I registered as user, but i could not create app, actually idk what was the name for app ))

Unfortunately I'm not quite sure what you mean by that. Could you give me a bit more information; as in, have you created the app yet? If you went through the process of creating an app, it will be listed at https://developer.tdameritrade.com/user/me/apps. Click on the app's name to expand details, and the APIKEY is under the Consumer Key section.

If you didn't create one, there will be a "New App" button (or something to that effect). Name it whatever you'd like, and if you are only deploying locally just keep the callback url the same.

If you do have any more questions, it would be better if you made an issue on https://github.com/samjtro/go-tda; that way if other people have similar Qs I can reference it there.

tanryberdi commented 2 years ago

@samjtro finally, i could ran )) about this issue, I investigated about the problem, and I think our problem is in FRAMEToDataSlice method we need to set values for VWAP & MACD : we need to update like following :

for _, x := range df {
        d1 := DATA{
            Close:  x.Close,
            Hi:     x.Hi,
            Lo:     x.Lo,
            Volume: x.Volume,
            VWAP: x.VWAP,
            MACD: x.MACD,
        }

        d = append(d, d1)
    }

to implement above one : we need to update FRAME struct in github.com/samjtro/go-tda@v0.8.2/data/data.go, need to add VWAP & MACD fields to that struct, after that I think we can set and take correct values for VWAP & MACD; If you create issue in github.com/samjtro/go-tda about that point you can assign to me. I will try to implement it. What do you think, am i right?

tanryberdi commented 2 years ago

@samjtro wdyt man?

samjtro commented 2 years ago

@samjtro wdyt man?

Hey, so sorry for the delay I've been travelling a lot this last week.

I do not believe that this is the case. What I think is happening is that there is an issue with the Ema() function.

Consider:

Using go run . we can run the application with default settings applied, AAPL quotes daily over a 3 month period.

...
{Close:141.66 Hi:141.91 Lo:139.77 Volume:8.9116837e+07 PivotPoint:141.11333333333334 ResistancePoints:[142.45666666666668 143.25333333333333 144.59666666666666] SupportPoints:[140.3166666666667 138.97333333333336 138.1766666666667] SMA:141.04762666666664 RMA:136.49666666666667 EMA:135.82803371145806 RSI:-9.219263456090601 VWAP:141.11333333333334 MACD:0 Chaikin:0 BollingerBands:[162.13336333333345 141.04762666666664 162.13336333333322] IMI:0 MFI:0 PCR:0 OI:0} {Close:141.66 Hi:143.49 Lo:140.965 Volume:7.0207908e+07 PivotPoint:142.03833333333333 ResistancePoints:[143.11166666666665 144.56333333333333 145.63666666666666] SupportPoints:[140.58666666666664 139.51333333333332 138.06166666666664] SMA:141.06187666666665 RMA:138.42666666666665 EMA:725.9910396393668 RSI:0 VWAP:142.03833333333333 MACD:0 Chaikin:0 BollingerBands:[162.1333633333334 141.06187666666665 162.13336333333328] IMI:0 MFI:0 PCR:0 OI:0} {Close:137.44 Hi:143.422 Lo:137.325 Volume:6.7315328e+07 PivotPoint:139.39566666666664 ResistancePoints:[141.4663333333333 145.49266666666665 147.5633333333333] SupportPoints:[135.3693333333333 133.29866666666663 129.27233333333328] SMA:140.62199333333336 RMA:140.53 EMA:141.55141314046452 RSI:0.6661760812914537 VWAP:139.39566666666667 MACD:0 Chaikin:0 BollingerBands:[162.13336333333334 140.62199333333336 162.13336333333334] IMI:0 MFI:0 PCR:0 OI:0}
...

We observe that all of the functions are actually working properly; with the exception of MACD(). This is because MACD() (and the super broken Chaikin() function) both use the Ema() function, which I kind of wrote last minute as a conversion of the EMA() which returns a DataSlice instead of just manipulating one over and over again.

I didn't really test it very well, and I don't think it works properly, but besides it basically has the same functionality so I'm gonna use the correct EMA() function instead.

How it now works is as such:

// Default value for d is set to len 12, which is the first calculation we want for MACD
d2.EMA(26, wg1)

    for i, x := range d {
        for _, y := range d2 {
            macd := x.EMA - y.EMA
            d[i].MACD = macd
        }
    }

I pushed my changes but I'm having concurrency issues, will fix today. But I think this sufficienctly fixes the calculation.

samjtro commented 2 years ago

Partial fix in commit cd8bd14fb81479eb5868b8c529b408ed91ca7cc9