Git Product home page Git Product logo

Comments (91)

krangelov avatar krangelov commented on August 20, 2024 1

from gf-core.

krangelov avatar krangelov commented on August 20, 2024 1

from gf-core.

krangelov avatar krangelov commented on August 20, 2024 1

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Wow, this seems a lot more advanced than you let on! I will take some time to better understand how it works.

from gf-core.

odanoburu avatar odanoburu commented on August 20, 2024

Hi @krangelov, pretty cool work! I remember hearing something about using SQLite for this work, which would give you transactions for free and also a platform-independent format (I'm assuming it would take the role of the .ngf files). Have you decided against it? I'm just curious and would love to contribute (I don't think I can, but I'll follow the discussion and if it turns out I can I'll say something!)

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

aarneranta avatar aarneranta commented on August 20, 2024

from gf-core.

odanoburu avatar odanoburu commented on August 20, 2024

@krangelov I see, thanks for the explanation! and daison looks interesting indeed!

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I figure the best way I can start contributing to this effort is to be your first user! Which probably means I will have lots of annoying setup questions. To start with, I tried to build the runtime using the instructions from the old C runtime, which failed. I set up a CI workflow here which documents what I tried and also contains the current error I get. I'm happy to contribute to other kinds of documentation (as library docs and how-to guides) as I learn more.

Going forward, we should add whatever testsuite you have (mentioned earlier) to this workflow.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

To be explicit, I am trying:

autoreconf -i
./configure
make

and getting:

make[1]: Entering directory '/home/runner/work/gf-core/gf-core/src/runtime/c'
  CXX      pgf/db.lo
  CXX      pgf/text.lo
  CXX      pgf/pgf.lo
  CXX      pgf/reader.lo
make[1]: *** No rule to make target 'pgf/expr.cxx', needed by 'pgf/expr.lo'.  Stop.
make[1]: Leaving directory '/home/runner/work/gf-core/gf-core/src/runtime/c'

What am I doing wrong?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Thanks Krasimir, the CI workflow now builds everything and runs the tests successfully.
I will keep poking around to understand things better. Probably the next thing I can work on is adding more tests.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

most of the code in the Python binding is commented out. It compiles, but
the only thing that you can do there is to load a grammar. I only needed
that in order to test that it is possible to load the runtime dynamically.

As far as I can see, nothing is commented out in src/runtime/python/pypgf.c?

I added a simple test to load the basic.pgf file from Python:

import pgf
import sys

sys.stdout.write("loading...")
sys.stdout.flush();
gr = pgf.readPGF("../haskell/tests/basic.pgf")
sys.stdout.write("\n")

but I get a segmentation fault. The CI output is here, and I get the same locally.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Ah, thanks for the clarification!

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Hi Krasimir,
I'm not experienced in C++, but I think this work is so important to the future of GF that it is worth me taking time to learn C++ itself as well as the internal details of how this new runtime works. I might be slow initially, but perhaps you can think of some simpler tasks to give me now, to help me get up to speed?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Great, I will start working on getting these implemented.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Checklist for Python bindings:

Reading from file

  • pgf_readNGF
  • pgf_bootNGF

Restore from old code

  • PGF_getAbstractName
  • PGF_getCategories
  • PGF_getFunctions
  • PGF_functionsByCat

Require marshalling of abstract expressions

  • PGF_functionType
  • PGF_getStartCat
  • Expr_repr
  • pgf_readExpr
  • Type_repr
  • pgf_readType

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

So I have managed to implement the simpler functions for getting categories/functions by restoring old code, but one thing I can't figure out is how the error handing is supposed to work. Namely, the GuExn* err parameter seems to have disappeared from functions such as pgf_iter_categories.

For now the old error-handling code is commented out like so:

static PyObject*
PGF_getCategories(PGFObject *self, void *closure)
{
    PyObject* categories = PyList_New(0);
    if (categories == NULL)
        return NULL;

    // GuPool* tmp_pool = gu_local_pool();
    //
    // Create an exception frame that catches all errors.
    // GuExn* err = gu_new_exn(tmp_pool);

    PyPGFClosure clo = { { pgf_collect_cats }, self, categories };
    pgf_iter_categories(self->pgf, &clo.fn);

    // if (!gu_ok(err)) {
    //     Py_DECREF(categories);
    //     gu_pool_free(tmp_pool);
    //     return NULL;
    // }
    //
    // gu_pool_free(tmp_pool);
    return categories;
}

Any pointers here?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Ok, what about the cases below? It seems possible that PyString_FromString and PyList_Append can fail. Do you think it's safe to ignore those too?

static void
pgf_collect_funs(PgfItor* fn, PgfText* key, void* value)
{
    PgfText* name = key;
    PyPGFClosure* clo = (PyPGFClosure*) fn;

    PyObject* py_name = NULL;

    py_name = PyString_FromString(name->text);
    if (py_name == NULL) {
        // gu_raise(err, PgfExn);
        goto end;
    }

    if (PyList_Append((PyObject*) clo->collection, py_name) != 0) {
        // gu_raise(err, PgfExn);
        goto end;
    }

end:
    Py_XDECREF(py_name);
}

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Thanks Krasimir. On the subject of strings, does this look right to you?:

static PyObject*
PGF_functionsByCat(PGFObject* self, PyObject *args)
{
    const char* c;
    if (!PyArg_ParseTuple(args, "s", &c))
        return NULL;

    const size_t s = strlen(c);

    PgfText* catname = (PgfText*) alloca(sizeof(PgfText)+s);
    strcpy(catname->text, c);
    catname->size = s;

    ...

I need to read the parameter from args into a PgfText (previously PgfCId). But PyArg_ParseTuple wants to put it into a char*, so I have to do this allocation + copy. I'm guessing strlen also works on null-terminated strings, but maybe here this is the correct solution since the string in the arguments not coming from the runtime?

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I've starting working on the marshalling in the Python bindings (see 3ecb937). I think it's starting to make sense, but I'd like to run a few things by you.

  • What should TypeObject contain (see expr.h)? Currently I have just a PgfText* in there for the type, but it needs more structure for the hypotheses etc. But am I correct in that we do not need a PgfType* in there anymore?
  • Next, when building a type object to return in the unmarshaller (see dtyp in marshaller.c), I am allocating a TypeObject and then returning it by casting it to a PgfType. Is that right?
  • Then, when using something which is created by the unmarshaller (see pgf_readType in pypgf.c), I just cast it back to a TypeObject* again. I think this aligns with what you wrote in the documentation, but would appreciate if you you confirm this.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

In TypeObject, I would put something like:

typedef struct {
    PyObject_HEAD
    PyList* hypos;
    PyString *cat;
    PyList* exprs;
} TypeObject;

In this way all fields use native Python types. The list hypos should
contain tuples with binding type, variable and the type of the variable.
The list exprs should contain objects of type ExprObject.

Understood!
What about the member PyObject* master, is this still relevant? I only half understood what its purpose is/was.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

The marshalling and types and expressions in the Python bindings is almost done. One thing in particular which I don't understand is the treatment of single quotes in identifier names.

In the Haskell tests you have:

TestCase (assertEqual "unicode names 2" (Just "ab") (fmap (showExpr []) (readExpr "'ab'")))

Which indicates that the outer quotes around the identifier are dropped when not needed.
In Python I have this test as:

def test_readExpr_efun_str_unicode_2():
    assert pgf.showExpr([], pgf.readExpr("'аb'")) == "аb"

However this fails, but passes with ... == "'ab'". So in Python the outer quotes aren't being dropped. I would expect this behaviour to come from the runtime; we are both using pgf_print_expr and pgf_read_expr rather directly, and I don't see that you do anything in particular for quotes in the Haskell bindings. Any idea where this difference could come from?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Hmm no I can't reproduce that:

>>> import pgf
>>> pgf.showExpr([], pgf.readExpr("'аb'"))
"'аb'"
>>> pgf.showExpr([],pgf.readExpr("'аb'"))
"'аb'"
>>> str(pgf.readExpr("'аb'"))
"'аb'"

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Transaction interface in Python

I'm thinking about how the Python API for transactions should be designed. In Haskell, you create a transaction with createFunction. Although it is implemented in terms of pgf_create_function, the latter only gets executed when a transaction is run with modifyPGF, thanks to functionality and partial evaluation.

We can't do things this way in Python, so my idea is this. Calling createFunction in Python creates some new Transaction object, which is internally modelled as a list of tuples or whatever, but which crucially is not implemented in terms of pgf_create_function. It is only the implementation of modifyPGF which then reads the transaction object and converts it into calls to pgf_create_function and friends. Is this the way you imagined it working?

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Roadmap

I was talking with @harisont about her possibly contributing to this effort too. For this (and in general) I think it might be good to have a bit of a short-term roadmap of what should be worked on and with what priority. I guess the main goal is to get concrete linearisation working, and there are certainly some subgoals in order to reach that point.
Another related question is what other bindings we want to have and whether it's worth working on them now. Specifically, I would like to eventually have bindings from Type/JavaScript and I know there are others who would like this too. But is it premature to start on this now?

Concretely we've been discussing these possibilities:

  1. Use the new Haskell bindings in an actual application (which is currently using the old Haskell runtime). This will require at least linearisation, generation, and morphological analysis.
  2. Split work on the Python bindings between me and her.
  3. She continues on the Python bindings and I look into starting work on JavaScript bindings.

Any thoughts on this?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

harisont avatar harisont commented on August 20, 2024

I'll try to help with the Java bindings then.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

The relationship between revision numbers and names is still a little unclear to me. Let me see if I understand this correctly.

  1. pgf_clone_revision is used to create a new "branch" with a new revision number, and an optional name which is implicitly master if omitted.
  2. Changes made with pgf_create_function etc. are made to a specific revision but not active until they are committed with pgf_commit_revision.
  3. If a revision has many pending changes and for some reason the commit fails, then none of them are applied.
  4. The API provides no way of knowing if a revision has uncommitted changes.
  5. Since every API function takes a revision as an argument, you can easily use different versions of a PGF side by side if you keep track of the revision numbers.
  6. pgf_checkout_revision gives you the revision number for a branch name. It is just a lookup and does not change the state of anything (like git checkout does).
  7. pgf_free_revision will remove a revision from memory, such that if you try to use that revision number after freeing it you will get an error.
  8. If two revisions are created from the same original one, and then both committed, both will be usable by their revision number, and the branch name will point to whichever was committed last. See pseudocode:
    (db, rev0) = new_ngf(...)
    rev1 = clone_revision(db, rev0)
    rev2 = clone_revision(db, rev0)
    create_function(db, rev0, "f0")
    create_function(db, rev1, "f1")
    create_function(db, rev2, "f2")
    commit_revision(db, rev1)
    commit_revision(db, rev2)
    assert get_functions(db, rev0) == [] // f0 was never committed
    assert get_functions(db, rev1) == ["f1"]
    assert get_functions(db, rev2) == ["f2"]
    assert checkout_revision("master") == rev2 // rev2 was committed after rev1
    

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Changes are always active but only for the revision in which they are done.

Ok, so the only effect of pgf_commit_revision is to update the link between branch name and revision number. If you only use only revision numbers and not names then you can basically ignore the commit/checkout functions.

This is not hard to add but why do we need it?

If changes are always active then it's not needed, I was assuming that there was such a thing as inactive/pending changes on a revision.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Thanks for all the documentation!
One thing that still bothers me though is that you refer to revisions being transient or persistent, but there is no way to query that information in the API. To me it sounds like a revision is basically persistent if there is some branch name attached to it. But if I have just a revision number, I cannot find out if it is persistent or not. Another seeming omission in the API is that there is no way to get all the available branch names — this actually would allow me to find out if a revision was persistent, by calling pgf_checkout_revision on all branch names and comparing the revision numbers.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

So immediately after this call:

r2 = clone(db, r1)

r1 and r2 are in a sense both linked to master because:

  • If I call checkout(db, "master") I get r1
  • If I call commit(db, r2) followed by checkout(db, "master"), then I get r2

What happens if you call clone on an already transient revision?

r2 = clone(db, r1)
r3 = clone(db, r2)

Will the runtime forbid it?

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Since the introduction of ipc.cxx (bdd84f1) I am getting this error:

ImportError: /usr/local/lib/libpgf.so.0: undefined symbol: shm_open

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Confirmed, thanks!

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Some questions about the probability functions:

  • pgf_function_prob returns positive infinity for a non-existent function; wouldn't 0 or some error make more sense? Maybe the runtime doesn't need to change, but I feel like the bindings should at least handle this better (e.g. raising KeyError in Python)
  • pgf_category_prob seems to always return 0, regardless of whether the category exists or not. I added two (failing) test cases in Haskell for this.
  • In the Haskell probability tests you are just checking for equality with pi, but because of floating-point imprecision I am having to use math.isclose() in Python. In createFunction I am reading the probability with PyArg_ParseTuple(args, "f", &prob) into a prob_t and in functionProbability I am converting it back with PyFloat_FromDouble((double)prob), which seems logical to me. I don't understand why it works in Haskell without any loss of precision though.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

In order to get something that is not zero you need to compile the grammar with an actual .probs file.

But how come the behaviour is different for non-existant functions and categories?

In Haskell I used the type double and then everything is fine.

I guess you mean Float here.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

So far I've been doing everything inside Docker. Today I tried to install the runtime directly in macOS and had some problems.
I'm following the old instructions, namely:

$ glibtoolize
$ autoreconf -i
$ ./configure
$ make
$ make install

And the make step gives me:

/Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
  CXX      pgf/libpgf_la-db.lo
In file included from pgf/db.cxx:6:
In file included from pgf/data.h:58:
pgf/db.h:115:26: error: unknown type name 'thread_local'
extern PGF_INTERNAL_DECL thread_local DB_scope *last_db_scope __attribute__((tls_model("initial-exec")));
                         ^
pgf/db.h:115:47: error: expected ';' after top level declarator
extern PGF_INTERNAL_DECL thread_local DB_scope *last_db_scope __attribute__((tls_model("initial-exec")));
                                              ^
                                              ;
In file included from pgf/db.cxx:6:
In file included from pgf/data.h:60:
pgf/vector.h:11:16: error: a space is required between consecutive right angle brackets (use '> >')
ref<PgfVector<A>> vector_new(size_t len)
               ^~
               > >
pgf/vector.h:13:20: error: a space is required between consecutive right angle brackets (use '> >')
    ref<PgfVector<A>> res = PgfDB::malloc<PgfVector<A>>(len*sizeof(A));
                   ^~
                   > >
pgf/vector.h:13:54: error: a space is required between consecutive right angle brackets (use '> >')
    ref<PgfVector<A>> res = PgfDB::malloc<PgfVector<A>>(len*sizeof(A));
                                                     ^~
                                                     > >
pgf/vector.h:22:43: error: a space is required between consecutive right angle brackets (use '> >')
    ref<C> res = PgfDB::malloc<PgfVector<A>>(offset+len*sizeof(A)).as_object()-offset;
                                          ^~
                                          > >
pgf/vector.h:28:35: error: a space is required between consecutive right angle brackets (use '> >')
ref<A> vector_elem(ref<PgfVector<A>> v, size_t index)
                                  ^~
                                  > >
In file included from pgf/db.cxx:6:
In file included from pgf/data.h:61:
pgf/namespace.h:23:19: warning: alias declarations are a C++11 extension [-Wc++11-extensions]
using Namespace = ref<Node<V>>;
                  ^
pgf/namespace.h:23:29: error: a space is required between consecutive right angle brackets (use '> >')
using Namespace = ref<Node<V>>;
                            ^~
                            > >
In file included from pgf/db.cxx:6:
In file included from pgf/data.h:62:
pgf/expr.h:33:23: error: a space is required between consecutive right angle brackets (use '> >')
        ref<PgfVector<PgfHypo>> hypos;
                             ^~
                             > >
pgf/expr.h:34:26: error: a space is required between consecutive right angle brackets (use '> >')
    ref<PgfVector<PgfExpr>> exprs;
                         ^~
                         > >
pgf/expr.h:283:44: error: a space is required between consecutive right angle brackets (use '> >')
void pgf_context_free(ref<PgfVector<PgfHypo>> hypos);
                                           ^~
                                           > >
In file included from pgf/db.cxx:6:
pgf/data.h:82:34: error: a space is required between consecutive right angle brackets (use '> >')
    ref<PgfVector<ref<PgfEquation>>> defns;
                                 ^~
                                 > > 
pgf/data.h:92:23: error: a space is required between consecutive right angle brackets (use '> >')
        ref<PgfVector<PgfHypo>> context;
                             ^~
                             > >
pgf/db.cxx:301:35: error: use of undeclared identifier 'errno'
            throw pgf_systemerror(errno, filepath);
                                  ^
pgf/db.cxx:305:24: error: use of undeclared identifier 'errno'
            int code = errno;
                       ^
pgf/db.cxx:314:28: error: use of undeclared identifier 'errno'
                int code = errno;
                           ^
pgf/db.cxx:329:20: error: use of undeclared identifier 'errno'
        int code = errno;
                   ^
pgf/db.cxx:334:15: error: use of undeclared identifier 'pthread_rwlock_init'
    int res = pthread_rwlock_init(&rwlock, NULL);
              ^
pgf/db.cxx:337:20: error: use of undeclared identifier 'errno'
        int code = errno;
                   ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
1 warning and 20 errors generated.
make[1]: *** [pgf/libpgf_la-db.lo] Error 1
make: *** [all] Error 2

I've tried updating/reinstalling all the dependencies, and using git clean to clear up any old generated files from src/runtime/c, but it has no effect.
I'm using the latest macOS 11.6, the output of g++ --version gives:

Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
Apple clang version 13.0.0 (clang-1300.0.29.3)
Target: x86_64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

A bit of searching makes me think I need to put the flag -std=c++11 somewhere, but I don't really understand the build system used here and don't know where to put it.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Thanks for the pointer. It made some difference in that I no longer get these kinds of errors:

pgf/data.h:92:23: error: a space is required between consecutive right angle brackets (use '> >')
        ref<PgfVector<PgfHypo>> context;
                             ^~
                             > >

But I still get the others, and some new ones.
No matter, I understand this is lower priority and it's not blocking me from working either. But I thought I might as well report how it went.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

So in commit 1401a6d I have solved most of the macOS compilation problems, except this:

pgf/db.cxx:887:64: error: use of undeclared identifier 'MREMAP_MAYMOVE'
                (malloc_state*) mremap(ms, old_size, new_size, MREMAP_MAYMOVE);

It seems that mremap is not supported in macOS at all, see here for example:
https://stackoverflow.com/questions/3521303/is-there-really-no-mremap-in-darwin

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Here's a fix for this from a Bitcoin repository, perhaps something similar can be used here:
libbitcoin/libbitcoin-blockchain@e23b6e0#diff-92ae62ce6f8e61467227bcbd38ddb2876646ae15e36f5a08a91c41acfdb8e84dL87

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Will you try to make the fix?

Sure

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

In the process I've come across another portability problem, namely the initialisation of flexible array members, which is apparently a GCC extension:

pgf/pgf.cxx:34:34: error: initialization of flexible array member is not allowed
PgfText master = {size: 6, text: {'m','a','s','t','e','r',0}};

References:

This seems to be used in two places, for the definitions of master in pgf.cxx and wildcard in expr.cxx. I've tried a few things but nothing fits neatly in the current code. My ideas:

  • There is some library-wide init function which can allocate the memory and write the values for these PgfText "constants".
  • The constants are not stored as a PgfText's but as two separate values:
    size_t master_size = 6;
    char master_text[] = {'m','a','s','t','e','r',0};
    
    and you have to do a little more work when comparing stuff with them.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Another issue related to the use of unsigned ints and EOF:

uint32_t PgfExprParser::getc()
{
    uint32_t ch = pgf_utf8_decode((const uint8_t **) &pos);
	if (pos - ((const char*) &inp->text) > inp->size) {
		ch = EOF;
	}
    return ch;
}
pgf/expr.cxx:368:7: error: case value evaluates to -1, which cannot be narrowed to type 'uint32_t' (aka 'unsigned int') [-Wc++11-narrowing]
        case EOF:
             ^

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Also:

ld: library not found for -lrt

Removing it from libpgf_la_LIBADD in Makefile.am seems to allow everything to compile.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I've pushed my current changes (WIP) to the branch majestic-macos, relevant diff is here: 8c721e0...majestic-macos

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

librt is needed ipc.cxx but that module is not used yet. That is why removing it worked.

From what I understand, whatever is provided by librt on Linux is included in standard libraries on macOS, so it's just a case of the flag itself not being needed on macOS.

https://stackoverflow.com/questions/34379290/library-rt-not-found-on-mac-while-configuring-sfft-sparse-fast-fourier-transfo

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Well, compiling now works on macOS (yay!), but readPGF immediately gives me a segmentation fault (boo). I'll see what I can find out by debugging.

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I compiled and run a minimal example. Putting all details here for reference.

Code

#include "pgf/pgf.h"

int main () {
  PgfRevision rev;
  PgfExn err;
  PgfDB *obj = pgf_read_pgf("/Users/john/repositories/GF/gf-core-majestic/src/runtime/haskell/tests/basic.pgf", &rev, &err);
}

Command

clang -lpgf -L/usr/local/lib main.c
lldb -o run ./a.out

Output

Process 81037 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10016d058)
    frame #0: 0x000000010012c7f2 libpgf.0.dylib`PgfDB::~PgfDB(this=0x0000000100404100) at db.cxx:368:17 [opt]
   365 	PgfDB::~PgfDB() {
   366 	    if (ms != NULL) {
   367 	        size_t size =
-> 368 	            ms->top + chunksize(ptr(ms,ms->top)) + sizeof(size_t);
   369 	
   370 	        munmap(ms,size);
   371 	    }
Target 0: (a.out) stopped.

Backtrace

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10016d058)
  * frame #0: 0x000000010012c7f2 libpgf.0.dylib`PgfDB::~PgfDB(this=0x0000000100304100) at db.cxx:368:17 [opt]
    frame #1: 0x000000010012f971 libpgf.0.dylib`::pgf_read_pgf(fpath=<unavailable>, revision=<unavailable>, err=0x00007ffeefbff908) at pgf.cxx:70:9 [opt]
    frame #2: 0x0000000100003f2c a.out`main + 28
    frame #3: 0x00007fff2046bf3d libdyld.dylib`start + 1
    frame #4: 0x00007fff2046bf3d libdyld.dylib`start + 1

Since this doesn't happen in Linux (inside Docker), I first suspected that the culprit is this workaround for mremap in db.cxx line 887:

#ifndef MREMAP_MAYMOVE
            if (munmap(ms, old_size) == -1)
                throw pgf_systemerror(errno);
            malloc_state* new_ms =
                (malloc_state*) mmap(0, new_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
#else
            malloc_state* new_ms =
                (malloc_state*) mremap(ms, old_size, new_size, MREMAP_MAYMOVE);
#endif

However I a set breakpoint there which did not get triggered, so that can't be the problem?
The backtrace mentions pgf.cxx:70, which is this:

69    if (db != NULL)
70        delete db;

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Slightly more investigation reveals that the read itself isn't working. If I comment out that destructor code, pgf_read_pgf gives me a PGF_EXN_SYSTEM_ERROR with code 9 (Bad file number).

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Also, loading from NGF with pgf_read_ngf seems to work perfectly well.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I've stepped through in the debugger but I cannot see anything going wrong with the call to mmap. The check with MAP_FAILED immediately afterwards is false.
That page you linked is outdated, plus seems to be for iOS not macOS. When I run man mmap on my system it says:

     MAP_ANONYMOUS     Synonym for MAP_ANON.

I couldn't find the documentation online, so here's a gist of it: man mmap

It seems to be this call in PgfReader::read_pgf which is causing the crash:

read_abstract(ref<PgfAbstr>::from_ptr(&pgf->abstract));

I know that read_absract is never reached. When I try to print pgf->abstract in the debugger I get the message "Couldn't lookup symbols" (maybe I need to recompile the runtime with/out some flags).

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Thanks! Either I was doing something wrong before, or turning off optimisation has changed things a little, such that now it seems that the call to

    abstract->funs = read_namespace<PgfAbsFun>(&PgfReader::read_absfun);

in PgfReader::read_abstract is the source. At some point it seems to "crash" and then the destructor of PgfDB suddenly gets called:

Process 99205 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
    frame #0: 0x0000000100140a1c libpgf.0.dylib`PgfReader::read_absfun(this=0x00007ffeefbff888) at reader.cxx:382:9
   379 	ref<PgfAbsFun> PgfReader::read_absfun()
   380 	{
   381 	    ref<PgfAbsFun> absfun =
-> 382 	        read_name<PgfAbsFun>(&PgfAbsFun::name);
   383 	    absfun->ref_count = 1;
   384 	    ref<PgfExprFun> efun =
   385 	        ref<PgfExprFun>::from_ptr((PgfExprFun*) &absfun->name);
Target 0: (a.out) stopped.
(lldb) n
Process 99205 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x100189058)
    frame #0: 0x000000010012a7f7 libpgf.0.dylib`PgfDB::~PgfDB(this=0x00000001003044d0) at db.cxx:369:17
   366 	PgfDB::~PgfDB() {
   367 	    if (ms != NULL) {
   368 	        size_t size =
-> 369 	            ms->top + chunksize(ptr(ms,ms->top)) + sizeof(size_t);
   370 	
   371 	        munmap(ms,size);
   372 	    }
Target 0: (a.out) stopped.

No, I know nothing about emulating macOS in Ubuntu. A quick search seems to indicate that it's actively discouraged by Apple. You could test things in a GitHub Actions workflow (which I will set up for macOS anyway) although testing by pushing & waiting is hardly optimal either. Perhaps we should schedule a meeting and debug together.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

I realized what is probably happening if indeed you actually go to the munmap/mmap part of the code.

When we have a file (i.e. fd > 0), then munmap/mmap works since the data is stored in the file. On the other hand when you have fid < 0 then munmap is just a more efficient version of free, and mmap becomes equivalent to malloc. In that case, of course, after free+malloc we will lose all the data.

On Mac the case when fd < 0 should be treated differently. In PgfDB::PgfDB the initial block should be allocated with malloc. In PgfDB::malloc_internal, instead of munmap+mmap we should use realloc().

I understand what you're suggesting and tried to make the changes in the majestic-macos branch, however there's a compilation error I can't understand:

pgf/db.cxx:331:28: error: no matching function for call to 'malloc'
      ms = (malloc_state*) malloc(file_size); // doesn't compile
                           ^~~~~~

More generally, what is the reason that this needs to be different for macOS?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Right, thanks! This seems to have worked. I'm now getting a lot further and pass all the Haskell "basic" tests. Now I'm getting a new error:

transactions: branchPGF: invalid argument (Invalid argument)

but I will need to debug this further. Could it be related to the call to mmap in ipc.cxx?

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

If I want to use the runtime from C, surely I don't need to implement my own un/marshaller? I see there are implementations of all the marshalling functions in expr.cxx, but these are not exposed in any way.
Maybe if one passes NULL when an unmarshaller is needed, it can/should use these internal implementations?

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

The cause behind the 'Invalid argument' error mentioned above seems to be this line in PgfDB::sync:

int res = msync((void *) ms, size, MS_SYNC | MS_INVALIDATE);

Which returns -1. Any idea how this should be changed for macOS (when ms is malloc'ed)?

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

For reference, macOS man page for msync: https://gist.github.com/johnjcamilleri/1cdc73d25420302035fb40df1aac2d75#file-msync

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Any idea how this should be changed for macOS (when ms is malloc'ed)?

I tested not calling mysnc at all in this case, and seems to work on all platforms (see CI here). But I'm not sure if it's the right solution.

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

krangelov avatar krangelov commented on August 20, 2024

from gf-core.

johnjcamilleri avatar johnjcamilleri commented on August 20, 2024

Ok, thanks for the clarifications!

I haven't anyone yet who would like to use the runtime from C :-)

Haha fair enough. I did this myself for testing purposes, in order to exclude the bindings themselves as a source of problems, but actually I managed to avoid using any functions that required un/marshalling.

from gf-core.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.