bugGNU Octave - Bugs: bug #44653, bolling.m in financial pkg does...

 
 

bug #44653: bolling.m in financial pkg does not use width parameter, gives incorrect result

Submitted by:  None
Submitted on:  Sat 28 Mar 2015 03:45:57 AM UTC  
 
Category: Octave Forge PackageSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Adam ThompsonOriginator Email: -unavailable-
Open/Closed: ClosedRelease: 3.8.1
Operating System: Mac OS

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 03 May 2015 10:52:58 PM UTC, comment #8:

You don't have to modify the history on the remote, you simply move back locally to where the history diverged and continue work from there. Simply abandon the extra heads on the remote. If you really want, you can ssh into the SF servers with your account, and use 'hg strip' there.

Also, there's no need to make pull requests via the SF interface. It only provides others with a list of commands to run locally to perform the merge, but even if it supplied a merge button à la git, we wouldn't use it as we favour a more linear history.

I have merged the four commits that fix the bug reported here as well as other improvements. I'm closing this bug as fixed.

About the new function amerbinary, I'm still not convinced. I saw that document you are linking before but that is on your own website and written for you. Google gives me nothing (just non-sense pages of what can only be automatically generated text https://support.google.com/webmasters/answer/answer.py?answer=2721306 ) and even scholar returns very few hits, most of them not even on journals. Maybe I'm missing something here (probably, finance is far from my field of research) but this doesn't seem like something that would fit on the general finance package (if you wish to continue this discussion, it is better if we do so on a separate bug/patch report).

Carnë Draug <carandraug>
Project Member
Sun 03 May 2015 05:58:26 PM UTC, comment #7:

I was unable to figure out how to modify the commit history on remote, so I deleted the old fork and made a new one. Please reject the previous merge requests.

Here are the new change sets for the new fork: https://sourceforge.net/u/azimzadeh/octave-financial/

- [8f6477] bolling.m: now uses width parameter (bug #44653)
- [7ab481] bolling.m: bands have the same colors; asset price is displayed alongside bands
- [06f4e9] bolling.m: added tests

There are also two unrelated change sets. The first centralizes error checking:

- [0ebe45] bls*.m, binprice.m: centralized error checking; conformed email addresses in headers

The second adds amerbinary to price american binary options.

- [12cca6] amerbinary.m: added routine to price american binary options

A document detailing the formula is available at http://parsiad.ca/teaching/American_Binary/ (HTML) or http://parsiad.ca/teaching/American_Binary/American_Binary.pdf (PDF)

I have not done a merge request on these changes, but let me know if I should.

Parsiad Azimzadeh <parsiad>
Sat 02 May 2015 07:43:22 PM UTC, comment #6:

Also, could you add a test for this to the function?

Carnë Draug <carandraug>
Project Member
Sat 02 May 2015 07:23:21 PM UTC, comment #5:

I forgot the verbatim on the previous comment which messed up indentation.

would become

Carnë Draug <carandraug>
Project Member
Sat 02 May 2015 07:21:55 PM UTC, comment #4:

Your cset http://hg.code.sf.net/u/azimzadeh/octave/rev/f44ed8cda0cd does more than just the fixing the bug. Could you split it into two separate commits.

Can you please address the following:

1. Follow octave coding guidelines. We follow the [GNU format](http://www.gnu.org/prep/standards/standards.html#Formatting). The main thing is leave a space between a function name and the opening brackets and continue the contents of brackets with that indentation. So this

plot( ...
x, asset(:), '-b', ...
x, avg(:), '-k', ...
x, avg(:)+s(:), '-r', ...
x, avg(:)-s(:), '-r' ...
);

would become

plot (x, asset(:), "-b", x, avg(:), "-k",
x, avg(:) .+ [-s(:) +s(:)], "-r")

Note the space in `plot (`, change to double quotess, and no need for `...`. I am also unsure that there's need to be constantly `(:)`

Also, are you also submitting the other cset, which adds a new function amerbinary, for review?

Carnë Draug <carandraug>
Project Member
Sat 02 May 2015 03:28:50 AM UTC, comment #3:

https://sourceforge.net/p/octave/financial/merge-requests/1/

Parsiad Azimzadeh <parsiad>
Thu 02 Apr 2015 02:22:08 AM UTC, comment #2:

Is there any chance you could submit a patch for this bug?

Carnë Draug <carandraug>
Project Member
Thu 02 Apr 2015 02:20:44 AM UTC, comment #1:

Adding Parsiad Azimzadeh, who has shown interest on becoming maintainer of the financial package, to this bug CC list.

Carnë Draug <carandraug>
Project Member
Sat 28 Mar 2015 03:45:57 AM UTC, original submission:

bolling.m does not make use of width parameter, gives the same result (width=1) no matter what value you put in. If you have <4 arguments, it should default to 2, but it defaults to 1. This is not the correct result for Bollinger Bands, which usually use 2 standard deviations for the upper and lower bands.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by parsiad (Posted a comment)
  • -unavailable- added by carandraug (proposed maintainer for financial package)
  • -unavailable- added by None (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 03 May 2015 10:52:58 PM UTCcarandraugStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Thu 02 Apr 2015 02:20:44 AM UTCcarandraugCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1