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

Submitter:  None
Submitted:  Sat 28 Mar 2015 03:45:57 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Adam Thompson Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.8.1
Operating System:  * Mac OS Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group 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>
Group Member
Sat 02 May 2015 07:23:21 PM UTC, comment #5: 

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


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")


Carnë Draug <carandraug>
Group 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>
Group Member
Sat 02 May 2015 03:28:50 AM UTC, comment #3: 
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>
Group 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>
Group 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 Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-05-03 carandraug StatusNone Fixed
        Open/ClosedOpen Closed
    2015-04-02 carandraug Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code