Opened 12 years ago

Last modified 11 years ago

#9739 closed defect

Forex trading - Expert — at Version 35

Reported by: mpatel Owned by: mvngu
Priority: blocker Milestone: sage-4.7.2
Component: doctest coverage Keywords: doctest scripts race condition unique filenames ptestlong -tp
Cc: drkirkby, jhpalmieri, leif, robertwb, jdemeyer Merged in:
Authors: Mitesh Patel Reviewers: Robert Bradshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by forextrading11)

http://www.forextradingcoach-simon.com/wp-content/uploads/2010/01/forex-trading.jpg

If you are an expert on forex trading you know already what it is to be a forex trader and the process that you have been in to is not easy but if you know how to handle tough tradings then it would be good for you because it will be who you are when you mature in this type of business.

Change History (37)

comment:1 Changed 12 years ago by mpatel

  • Description modified (diff)

William Stein suggests:

Could we use the tempfile module instead of using SAGE_TESTDIR.  The
tempfile module makes files and directories by default that are unique
and are *designed* to live on a fast filesystem, which gets cleaned
regularly.

sage: import tempfile 

Changed 12 years ago by mpatel

Frequency-sorted list of doctest file basenames (includes .py, .pyx, .pxi, .rst, .tex files). Not a patch.

comment:2 Changed 12 years ago by mpatel

I've attached a list of ~1500 unique basenames for the ~2500 files we doctest (give or take a handful).

comment:3 Changed 12 years ago by leif

  • Cc leif added

Changed 12 years ago by mpatel

Doctest with unique temporary files. Apply to scripts repo.

comment:4 Changed 12 years ago by mpatel

  • Authors set to Mitesh Patel
  • Cc drkirkby jhpalmieri added
  • Milestone set to sage-4.6
  • Status changed from new to needs_review

I've attached a patch. I haven't modified sage-test, since we're likely to phase it out at #9224.

comment:5 follow-up: Changed 12 years ago by leif

Hmmm, I'm not sure if people will like substituting the filename with the whole test command everywhere. (I have a wide screen, but...)

Can we please stop creating "hidden" doctest files? To me that doesn't make sense at all; especially but not limited to files in ~/.sage/*/.

Does the "For whitespace errors, see ..." still work (i.e., print the correct filename)?

The notion of "absolute filenames relative to ..." is a bit weird... (But thanks for renaming abs(). :) )

The rest seems ok; I've yet only looked at the patch though.

comment:6 Changed 12 years ago by leif

While printing the number of global and file iterations even if both are 1 isn't that useful, I'd like to see the current / appropriate timeout setting[s] printed in sage-ptest (i.e. SAGE_TIMEOUT for normal tests, and only SAGE_TIMEOUT_LONG when -long was given).

(In principle, files containing no examples marked # long time should IMHO not be tested with SAGE_TIMEOUT_LONG, but the normal timeout. In that case we should print both timeout settings, and perhaps also indicate with which a file is being tested.)

comment:7 in reply to: ↑ 5 Changed 12 years ago by leif

Replying to leif:

Hmmm, I'm not sure if people will like substituting the filename with the whole test command everywhere. (I have a wide screen, but...)

Ok, sorry, hard to read. The behavoir seems to be the same as before...

comment:8 follow-up: Changed 12 years ago by drkirkby

How should I best apply this:

drkirkby@hawk:~/2/sage-4.5.3.alpha2$ cd local/bin
drkirkby@hawk:~/2/sage-4.5.3.alpha2/local/bin$ hg qimport  http://trac.sagemath.org/sage_trac/raw-attachment/ticket/9739/trac_9739-unique_doctest_names.patch
adding trac_9739-unique_doctest_names.patch to series file
drkirkby@hawk:~/2/sage-4.5.3.alpha2/local/bin$ hg push
abort: repository /space/rlm/sage-4.1.rc1/local/bin not found!

Something, somewhere seems to be looking for some directory of what I assume is Robert Millers.

Dave

comment:9 in reply to: ↑ 8 Changed 12 years ago by mpatel

Replying to drkirkby:

Something, somewhere seems to be looking for some directory of what I assume is Robert Millers.

What happens if you (re)move SAGE_LOCAL/bin/.hg/hgrc? The problem might be that when you're in SAGE_LOCAL/bin, you're invoking ./hg, instead of /usr/local/bin/hg.

comment:10 Changed 12 years ago by mpatel

  • Cc robertwb added

comment:11 follow-up: Changed 12 years ago by robertwb

Note that you don't want to create a race condition with two tests trying to create the same directory at the same time. Perhaps mangling "/" -> "." would be sufficient.

comment:12 in reply to: ↑ 11 Changed 12 years ago by mpatel

  • Reviewers set to Robert Bradshaw

Replying to robertwb:

Note that you don't want to create a race condition with two tests trying to create the same directory at the same time. Perhaps mangling "/" -> "." would be sufficient.

Thanks, Robert. How about adding the process ID, too, or instead? I'd also like to reduce the chance of races when we run multiple sage -t(p) commands simultaneously with the same DOT_SAGE. Or are there other potential races under this directory, e.g., in DOT_SAGE/gap?

comment:13 Changed 12 years ago by mpatel

  • Status changed from needs_review to needs_work

comment:14 Changed 12 years ago by drkirkby

Has there been any more thoughts on this?

I had a doctest failure today on my OpenSolaris machine hawk, which is almost certainly a result of this bug. This is using sage-4.6.rc0.

The following tests failed:

	sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/notebook/misc.py # 0 doctests failed
	sage -t  -long devel/sage/sage/plot/plot.py # 5 doctests failed

The likely reason for the first of these failures can be seen if we look further up the log.

sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/misc/misc.py
         [2.9 s]
sage -t  -long local/lib/python2.6/site-packages/sagenb-0.8.2-py2.6.egg/sagenb/notebook/misc.py
python: can't open file '/export/home/drkirkby/.sage//tmp/misc.py': [Errno 2] No such file or directory

         [0.2 s]

The log clearly shows a file misc.py being tested, followed by a second test with a different file also called misc.py. That file is then not found. It seems almost inevitable one test deleted the file needed by another test as they were both called misc.py and both tested around the same time.

Note this is the same machine on which the buildbot passed all doctests.

http://build.sagemath.org/sage/builders/hawk%20full/builds/8/steps/shell_6/logs/stdio

so it seems to be an intermittent problem. (I've also had all tests pass on this).

These doctests issues are really annoying, as one never knows if its a real Sage bug, or a doctest bug.

Dave

comment:15 Changed 12 years ago by jdemeyer

  • Cc jdemeyer added

comment:16 follow-up: Changed 12 years ago by mpatel

I'm planning to return to this soon, probably after 4.6 is out.

comment:17 in reply to: ↑ 16 Changed 12 years ago by jdemeyer

Replying to mpatel:

I'm planning to return to this soon, probably after 4.6 is out.

That would be great, because I have also hit this error quite a few times.

comment:18 Changed 12 years ago by jdemeyer

  • Keywords doctest scripts added
  • Milestone changed from sage-4.6 to sage-4.6.1
  • Priority changed from critical to blocker

comment:19 follow-up: Changed 12 years ago by drkirkby

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage. They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

I found that testing devel/sage/sage/libs/fplll/fplll.pyx (see #10195), would generate problems when I was testing two copies of Sage at the same time (slightly different versions). I don't know if this patch can handle that situation, but it would be good if it could.

Dave

comment:20 in reply to: ↑ 19 ; follow-up: Changed 12 years ago by leif

Replying to drkirkby:

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage.

Well this would definitely be a user error. You can always set DOT_SAGE or SAGE_TESTDIR (or whatever it is called) if you want to run multiple tests simultaneously in different shells, even in / with the same Sage installation.

They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

Definitely not, since this wouldn't work for site installations, where users usually have no write permissions under SAGE_ROOT.

I don't know if this patch can handle that situation, but it would be good if it could.

One could use Sage's PID, user and machine parameters etc. to try to create unique directories, or generally create "random" directories with mktemp (1) or mkdtemp(), but I think this would be an overkill, since the user can itself do such by setting one of the above variables.

comment:21 in reply to: ↑ 20 ; follow-up: Changed 12 years ago by drkirkby

Replying to leif:

Replying to drkirkby:

There is another problem, which could exist even if every file had a different name.

If one tests multiple instances of Sage serially, then since they both write to $HOME/.sage, failures can occur even if the file names of the doctests are unique to any one copy of Sage.

Well this would definitely be a user error. You can always set DOT_SAGE or SAGE_TESTDIR (or whatever it is called) if you want to run multiple tests simultaneously in different shells, even in / with the same Sage installation.

They need to be unique for any number of instances of Sage. I think testing under $HOME/.sage is a bit silly myself - it would be better to test under the directory where Sage is installed.

Definitely not, since this wouldn't work for site installations, where users usually have no write permissions under SAGE_ROOT.

I don't know if this patch can handle that situation, but it would be good if it could.

One could use Sage's PID, user and machine parameters etc. to try to create unique directories, or generally create "random" directories with mktemp (1) or mkdtemp(), but I think this would be an overkill, since the user can itself do such by setting one of the above variables.

I disagree. I don't think creating unique temporary files is overkill. It would be far less error prone. With a test you want to run many times, having a dozen copies of Sage around for test purposes is quite a sensible thing to do with multi-core machines. Havving to set DOT_SAGE for each is annoying, when a unique temporary file could be made.

Dave

comment:22 in reply to: ↑ 21 Changed 12 years ago by leif

Replying to drkirkby:

I disagree. I don't think creating unique temporary files is overkill.

I didn't say that, but it IMHO suffices to have unique names in the namespace of a ptest run.

It would be far less error prone. With a test you want to run many times, having a dozen copies of Sage around for test purposes is quite a sensible thing to do with multi-core machines. Havving to set DOT_SAGE for each is annoying, when a unique temporary file could be made.

As I said, you can automatically set up unique test directories by setting one of the above variables e.g. based on the "login" shell's PID, one per terminal / shell.

If multiple machines share the same filesystem, simply add e.g. the hostname, too.

comment:23 follow-up: Changed 12 years ago by robertwb

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

comment:24 in reply to: ↑ 23 ; follow-up: Changed 12 years ago by drkirkby

Replying to robertwb:

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

I'm not sure if you are agreeing with me or Leif there Robert - perhaps you can clarify.

I was going to suggest that we should be using /tmp, but I did not since I can see a disadvantage of it. NFS file systems have caused problems with doc tests failing, especially if they are mis-configured. As such, it would be better if a user tested Sage on the file system where it will be used. By using /tmp they might get a false sense of security. That said, using /tmp for temporary files has been the norm for years.

I personally think where reasonably practical, we should stop multiple instances of running Sage tests interfering with each other. Although Leif considers this a user error, it is one that a user might easily make.

But if it possible to avoid creating temporary files, then that should be done. But I would imagine that requires more changes than just adding a pid or hostname.

Dave

comment:25 follow-up: Changed 12 years ago by leif

OT: My make ptestlong of Sage 4.6 on Ubuntu 9.04 x86 somehow went into an infinite loop...

This never happened before (in dozens of builds), any idea?

(I started the complete build with make ptestlong, build succeeded and building the documentation went normal, but now I'm meanwhile at the tenth doctest run! Checked this with grep Doctesting ptestlong.log.)

comment:26 in reply to: ↑ 24 Changed 12 years ago by robertwb

Replying to drkirkby:

Replying to robertwb:

+1 to temp (per instance) directories. They would get cleaned up properly, and /tmp is almost always fast and local which is another plus.

(Really, we shouldn't have to be writing these files out at all...)

I'm not sure if you are agreeing with me or Leif there Robert - perhaps you can clarify.

Agreeing with drkirkby, we should use temp directories.

I was going to suggest that we should be using /tmp, but I did not since I can see a disadvantage of it. NFS file systems have caused problems with doc tests failing, especially if they are mis-configured. As such, it would be better if a user tested Sage on the file system where it will be used. By using /tmp they might get a false sense of security. That said, using /tmp for temporary files has been the norm for years.

Don't know if mktemp is POSIX, but it's widely available. Of course from Python you always have http://docs.python.org/library/tempfile.html Both are much better than manually specifying /tmp.

I personally think where reasonably practical, we should stop multiple instances of running Sage tests interfering with each other. Although Leif considers this a user error, it is one that a user might easily make.

I don't consider it a user error, and I also don't like filling .sage with lots of junk.

But if it possible to avoid creating temporary files, then that should be done. But I would imagine that requires more changes than just adding a pid or hostname.

True, and this ticket has been opened for far too long.

  • Robert

comment:27 in reply to: ↑ 25 Changed 12 years ago by leif

Replying to leif:

OT: My make ptestlong of Sage 4.6 on Ubuntu 9.04 x86 somehow went into an infinite loop...

This never happened before (in dozens of builds), any idea?

(I started the complete build with make ptestlong, build succeeded and building the documentation went normal, but now I'm meanwhile at the tenth doctest run! Checked this with grep Doctesting ptestlong.log.)

Ouch. I just noticed I had set SAGE_TEST_GLOBAL_ITER=1000 earlier in that shell...

(But all tests passed; I then aborted it during the 14th run.)

comment:28 follow-up: Changed 12 years ago by jdemeyer

What is the current status of this patch, do you consider it ready for review? If not, I'm willing to join in and help where needed.

comment:29 in reply to: ↑ 28 Changed 12 years ago by mpatel

Replying to jdemeyer:

What is the current status of this patch, do you consider it ready for review? If not, I'm willing to join in and help where needed.

It's not ready for review. I don't think I have the time to work on this in the next several days, so if you'd like to help, please do!

I think we can make a temporary directory for each run in sage-(p)test and pass it to sage-doctest, which should ensure the temporary files under the directory are unique. The existing doesn't quite yet do the latter, but it should be easy to modify it so it does (e.g., with full paths and/or pids, etc.).

comment:30 Changed 12 years ago by drkirkby

Using both a hostname and a PID should mean the filename is practically unique if people test on more than one computer using a shared drive. I don't think 'mktemp' will be unique on NFS shared drives, though the probability of a collision would then be very small indeed. But adding a hostname would reduce it even further.

But we need to be careful if using mktemp. Whilst many systems have it, the implementation is not the same on every system. I know Solaris works a bit different to Linux or OS X (I forget which). I know using something on Solaris with mktemp which would not work with Linux or OS X. (I forget which OS it was though). It seems HP-UX and Solaris differ too.

I would be very keen to use something that will work on AIX. There is a chance of IBM donating a quad core 4.5 GHz machine to the Sage project with AIX on it.

Anyway, whatever solution is used, I think it will be 1000x better than the current solution, but personally I'd like to see something that's unique to a machine and portable.

Dave

comment:31 Changed 11 years ago by jdemeyer

  • Milestone changed from sage-4.6.1 to sage-4.6.2

I'm still interested in working on this patch, but I don't expect it to be ready on time for 4.6.1.

comment:32 Changed 11 years ago by leif

While we are at it, I have some more comments on sage-ptest I'll also post here with an inline patch:

  • sage-ptest

    diff --git a/sage-ptest b/sage-ptest
    a b  
    8181    Returns true if the file should not be tested
    8282    """
    8383    if not os.path.exists(F):
     84        # XXX IMHO this should never happen; in case it does, it's certainly
     85        #     an error to be reported (either filesystem, or bad name specified
     86        #     on the command line). -leif
    8487        return True
    8588    G = abspath(F)
    8689    i = G.rfind(os.path.sep)
     90    # XXX The following should IMHO be performed in populatefilelist():
     91    #     (Currently, populatefilelist() only looks for "__nodoctest__".)
    8792    if os.path.exists(os.path.join(G[:i], 'nodoctest.py')):
    8893        printmutex.acquire()
    8994        print "%s (skipping) -- nodoctest.py file in directory"%abs(F)
     95        sys.stdout.flush()
    9096        printmutex.release()
    9197        return True
    9298    filenm = os.path.split(F)[1]
     
    95101        return True
    96102    if G.find(os.path.join('doc', 'output')) != -1:
    97103        return True
     104    # XXX The following is (also/already) handled in populatefilelist():
    98105    if not (os.path.splitext(F)[1] in ['.py', '.pyx', '.tex', '.pxi', '.sage', '.rst']):
    99106        return True
    100107    return False
     
    115122    for i in range(0,numiteration):
    116123        os.chdir(os.path.dirname(F))
    117124        command = os.path.join(SAGE_ROOT, 'local', 'bin', 'sage-%s' % cmd)
     125        # FIXME: Why call bash here? (Also, we use 'shell=True' below anyway.)
    118126        s = 'bash -c "%s %s > %s" ' % (command, filestr, outfile.name)
    119127        try:
    120128            t = time.time()
     
    161169        print sage_test_cmd(F[len(CUR)+1:])
    162170    else:
    163171        print abs(F)
     172    sys.stdout.flush()
    164173    if ol!="" and (not ol.isspace()):
    165174        if (ol[len(ol)-1]=="\n"):
    166175            ol=ol[0:len(ol)-1]
    167176        print ol
     177        sys.stdout.flush()
    168178    time_dict[abs_sage_path(F)] = finished_time
    169179    if XML_RESULTS:
    170180        t = finished_time
     
    192202        """.strip() % locals())
    193203        f.close()
    194204    print "\t [%.1f s]"%(finished_time)
     205    sys.stdout.flush()
    195206
    196207def infiles_cmp(a,b):
    197208    """
     
    231242                base, ext = os.path.splitext(F)
    232243                if not (ext in ['.sage', '.py', '.pyx', '.tex', '.pxi', '.rst']):
    233244                    continue
    234                 elif '__nodoctest__' in files:
     245                elif '__nodoctest__' in files: # XXX Shouldn't this be 'lfiles'?
     246                    # Also, this test should IMHO be in the outer loop (1 level).
     247                    # Furthermore, the current practice is to put "nodoctest.py"
     248                    # files in the directories that should be skipped, not
     249                    # "__nodoctest__". (I haven't found a single instance of the
     250                    # latter in Sage 4.6.1.alpha3.)
     251                    # "nodoctest.py" is handled in skip() (!), to also be fixed.
     252                    # -leif
    235253                    continue
    236254                appendstr = os.path.join(root,F)
    237255                if skip(appendstr):
     
    252270    argv = [X for X in argv if X[0] != '-']
    253271
    254272    try:
     273        # FIXME: Nice, but <NUMTHREADS> should immediately follow '-tp' etc.,
     274        #        i.e., be the next argument. We might have file or directory
     275        #        names that properly convert to an int...
    255276        numthreads = int(argv[1])
    256277        infiles = argv[2:]
    257278    except ValueError: # can't convert first arg to an integer: arg was probably omitted

(This is against Sage 4.6.1.alpha3.)

The comments all refer to inconsistencies; the only actual change is flushing the output since it currently comes in bursts, which is IMHO odd for watching it. I don't think this measurably slows down doctesting...

comment:33 follow-up: Changed 11 years ago by leif

Just for the record:

#10458 also patches sage-ptest to support IPython/Sage-style line continuations in doctests ("....: " rather than only "...").

comment:34 in reply to: ↑ 33 Changed 11 years ago by leif

Replying to leif:

Just for the record:

#10458 also patches sage-ptest to support IPython/Sage-style line continuations in doctests ("....: " rather than only "...").

Ooops, sorry, #10458 patches sage-doctest, not sage-ptest.

comment:35 in reply to: ↑ description Changed 11 years ago by forextrading11

  • Cc Marian Rosales added; drkirkby jhpalmieri leif robertwb jdemeyer removed
  • Component changed from doctest to build
  • Description modified (diff)
  • Keywords currency trading forex analysis forex trading online forex trading added; doctest scripts removed
  • Priority changed from blocker to trivial
  • Summary changed from Handle duplicate file basenames when testing multiple files in parallel to Forex trading - Expert
  • Type changed from defect to task

Replying to mpatel:

When we test

/path/to/foo.py

sage-doctest writes

SAGE_TESTDIR/.doctest_foo.py

runs the new file through python, and deletes it. This can cause collisions when we test in parallel multiple files with the same basename, e.g., __init__, all, misc, conf, constructor, morphism, index, tests, homset, element, twist, tutorial, sagetex, crystals, cartesian_product, template, ring, etc.

There's a similar problem with testing non-library files, which sage-doctest first effectively copies to SAGE_TESTDIR.

See sage-devel for background.

This ticket may help with some of the doctesting problems discussed on the Sage mailing lists. Related tickets: #9224, #9225, #9449.

Note: See TracTickets for help on using tickets.