Ticket #479 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Rendering fragment

Reported by: vzholudev Owned by: dmisev
Priority: blocker Milestone: Release v0.1.4
Component: [SI] NTN Version: v0.1.4
Keywords: Cc: clange, kohlhase, cmueller, dmisev
Blocked By: Blocking:
Due to close: Include in GanttChart: no
Dependencies: Due to assign:

Description

During preparing a demo, I encountered a strange problem. What I do in order to render a document doc: 1)Add notation source: new D(doc) 2)Render this document

As far as I understood, it collects notations from doc and all imported documents recursively

Rendering works fine. When I try to render a definition fragment from the same document using same notations(i.e. a renderer), then this fragment is not rendered properly as no notations were captured.

However, if I add notations source as new F(doc), then the symbols in a fragment are rendered prperly (i.e. those which have notations in doc)

The question is: is D class supposed to server as a notation source for fragments? Could it be a problem of URIs, since I resolve docs from TNTBase by providing uris strating with tntbase:

Is information is not sufficient, I can provide my code and test data.

Change History

  Changed 3 years ago by vzholudev

  • cc clange, kohlhase, cmueller, dmisev added

in reply to: ↑ description   Changed 3 years ago by cmueller

Replying to vzholudev:

The question is: is D class supposed to server as a notation source for fragments? Could it be a problem of URIs, since I resolve docs from TNTBase by providing uris strating with tntbase: Is information is not sufficient, I can provide my code and test data.

I think looking at your testdata would be helpful. The Doc option collects all notation definition to the left of a mathematical object. So this could cause a problem as not all notation definitions in the document might apply to your fragment.

follow-up: ↓ 5   Changed 3 years ago by dmisev

Christine, he talks about D which is different from Doc and works as he explained

it collects notations from doc and all imported documents recursively

Slava in order for it to work you need to add the notation source as collector.add(new D(doc)) (something like that), but please provide the testdata/code so I can check.

  Changed 3 years ago by nmueller

  • due_assign YYYY/MM/DD deleted
  • owner changed from nmueller to dmisev
  • component changed from System Implementation to [SI] NTN
  • due_close YYYY/MM/DD deleted
  • milestone set to Release v0.1.3

in reply to: ↑ 3   Changed 3 years ago by clange

Replying to dmisev:

Christine, he talks about D which is different from Doc and works as he explained

So much for naming guidelines ;-)

follow-up: ↓ 8   Changed 3 years ago by dmisev

  • status changed from new to assigned

Christine doesn't know about it because it was implemented on request by Michael.

How would you suggest to name it? I thought I should stay consistent to the previous names (F, IC, MD, etc) so I named it D, document containing theories.

  Changed 3 years ago by vzholudev

Here is my code how I want to render a fragment. Do I do it right? Please look at, it's very important to improve efficiency of AI_mashup demo.

I execute the last method in this list. fragment - is a string representation of a fragment I want to render. If fragment is the document that is under the path "path", then it gets rendered perfectly. Thanks

    public Document getXHTMLFromOmdoc(Document omdocDoc) throws IOException, ParsingException, XSLException {
        Document renderedDoc = renderer.render(omdocDoc);
        return XSLTUtil.transform(renderedDoc, null);
    }

    public Document getXHTMLFromOmdoc(String omdocContent, String path) throws IOException, ParsingException, XSLException {
        Document doc = getBuilder().build(omdocContent, TNTBaseResolver.getTNTBaseUri(path));
        return getXHTMLFromOmdoc(doc);
    }

    public Document renderWithNotations(String wholeDocument, String fragment, String path) throws IOException, ParsingException, XSLException, XmlException {
        ByteArrayInputStream is = new ByteArrayInputStream(wholeDocument.getBytes(XMLUtil.XML_STD_ENCODING));
        TNTBaseResolver resolver = new TNTBaseResolver(accessor);
        XSLTUtil.setResolver(resolver);

        if(!notationDocsPaths.contains(path)) {
            ncoll.addNotationSource(new D(is, TNTBaseResolver.getTNTBaseUri(path), true, null, resolver));
            Document docToCollect = getBuilder().build(is, TNTBaseResolver.getTNTBaseUri(path));
            ncoll.addNotationSource(new F(docToCollect));
            notationDocsPaths.add(path);
        }
        return getXHTMLFromOmdoc(fragment, path);
    }

    public Document renderWithNotations(String path) throws IOException, ParsingException, XSLException, XmlException {
        String content = accessor.getQueryRequestProcessor().getDocumentByPath(path);
        return renderWithNotations(content, content, path);
    }


    public Document renderFragmentWithNotations(String fragment, String path) throws XmlException, IOException, ParsingException, XSLException {
        String content = accessor.getQueryRequestProcessor().getDocumentByPath(path);
        return renderWithNotations(content, fragment, path);
    }

in reply to: ↑ 6   Changed 3 years ago by cmueller

Replying to dmisev:

Christine doesn't know about it because it was implemented on request by Michael. How would you suggest to name it? I thought I should stay consistent to the previous names (F, IC, MD, etc) so I named it D, document containing theories.

Don't worry, we will deal with the refactoring of class names in ticket:454. So please leave D for now. In today's meeting we said that this refactoring should be done in coordination with all other developers of JOMDoc or system's that use JOMDoc as changes to the interface will have an impact to these systems as well.

  Changed 3 years ago by vzholudev

It's not about class names. How am I supposed to collect notations from the current file and all imported ones? renaming won't solve my problem. And I would say that renaming is not so crucial as memory leak in NtnUtil? or the problem with rendering fragment.

Could anybody please post a code snippets how he/she renderes fragments? (in the way different to collecting all notation via F class). Or am I the only one who decided to use such features of JOMDoc?

@Dimitar, do you know whether I'm doing right?

  Changed 3 years ago by clange

I cannot offer help here, as I have only done rendered fragments with F notations before. However, I really can't imagine that there is a hidden dependency between the way notations are collected (e.g. using D) and the fragment/document to be rendered. Here, it would be helpful to inspect the whole notation collection after collecting with D, whether it contains all desired notation definitions for all desired symbols. Then, when a fragment is passed to the renderer, it should be treated like anything else: those symbols, to which notation definitions in the collection (which should be immutable at that point, as its construction has been finished in the past!) match, will be rendered.

  Changed 3 years ago by dmisev

Try turning on debugging output (Log.setAll()) and see whether that helps. Also it would be great if you give me access to the testdata (the input document and fragment). For rendering fragments you should directly use renderer.renderElement btw.

  Changed 3 years ago by vzholudev

Ok, finally got test data: Document from which we got a fragment and which we use as a notation source:  https://alpha.tntbase.mathweb.org/repos/ai-mashup/dmath/en/sets-operations.omdoc

Fragment I try to render:

<definition xmlns="http://omdoc.org/ns" for="#sseteq #sseteqOp #subset" xml:id="subset.def">
          <CMP xml:id="subset.def.p1">
            <p xml:id="subset.def.p1.p1"><idx><idt><term cd="sets-operations" name="subset" role="definiendum">subset</term></idt><ide index="default"><idp>subset</idp></ide></idx>: <om:OMOBJ xmlns:om="http://www.openmath.org/OpenMath"><om:OMA><om:OMS cd="highschool" name="defequiv"/><om:OMA><om:OMS cd="sets-operations" name="sseteq"/><om:OMV name="A"/><om:OMV name="B"/></om:OMA><om:OMA><om:OMS cd="mathtalk" name="foral"/><om:OMV name="x"/><om:OMA><om:OMS cd="mathtalk" name="imply"/><om:OMA><om:OMS cd="set1" name="inset"/><om:OMV name="x"/><om:OMV name="A"/></om:OMA><om:OMA><om:OMS cd="set1" name="inset"/><om:OMV name="x"/><om:OMV name="B"/></om:OMA></om:OMA></om:OMA></om:OMA></om:OMOBJ></p>
          </CMP>
        </definition>

I use renderElement() as well, and it still doesn't work.

Please have a look!

  Changed 3 years ago by dmisev

I've fixed the problem, but note that now renderElement directly modifies the given element. The problem was that the given fragment must not be detached, but in renderElement it was actually copied and thus detached, so when collecting the notations in D the context of the fragment was not available (the theory containing the fragment couldn't be found).

Please check and close the ticket if it works now.

  Changed 3 years ago by dmisev

  • status changed from assigned to closed
  • resolution set to fixed

Or reopen if it doesn't work.

  Changed 3 years ago by vzholudev

  • status changed from closed to reopened
  • resolution fixed deleted

I still can't get the appropriate rendering. My piece of code:

    public Document getXHTMLFromOmdoc(Document omdocDoc, boolean isFragment) throws IOException, ParsingException, XSLException {
        Document renderedDoc;
        if(isFragment) {
            renderedDoc = new Document((Element)renderer.renderElement(omdocDoc.getRootElement()).copy());
        } else {
            renderedDoc = renderer.render(omdocDoc);
        }
        return TNTBaseXSLTUtil.transform(renderedDoc, null);
    }

Before I execute:

            ncoll.addNotationSource(new D(is, TNTBaseResolver.getTNTBaseUri(path), true, null, resolver));

Where is is the InputStream? obtained from the whole document that contains the element to be rendered.

Am I doing smth wrong?

  Changed 3 years ago by dmisev

  • status changed from reopened to closed
  • resolution set to fixed

I did some hacking in order to make this working.. before you call renderElement, set the theory name in which the detached element is with

D.setTheoryName("theoryName");

See DTest.testFragmentRendering for an example.

follow-up: ↓ 18   Changed 3 years ago by vzholudev

  • status changed from closed to reopened
  • resolution fixed deleted

Finally, I've got time to look at this. From having a glance at the setTheoryName() method, I tend to think that this approach is not thread safe. And if we integrate JOMDoc into some web-application, then there is definitely a multi-threaded environment.

in reply to: ↑ 17   Changed 3 years ago by clange

Replying to vzholudev:

Finally, I've got time to look at this. From having a glance at the setTheoryName() method, I tend to think that this approach is not thread safe. And if we integrate JOMDoc into some web-application, then there is definitely a multi-threaded environment.

Indeed the setTheoryName solution is evil. IIUC the main problem here is that we somehow need the theory name but that we cannot allow for passing it down into the methods of D from the outermost call that renders the fragment, as that would require a redesign of a whole chain of method signatures.

But would it be possible, at least as a workaround, not to pass a detached fragment to JOMDoc but a fragment that still knows what document it is part of? Then the D implementation could walk to the parent elements of the fragment and finally discover the theory name. That would be less elegant and less efficient for TNTBase, but I think it would work. What I mean is, replace the current (pseudo code):

Element frag = GET document/path/to/fragment
// frag.getParent() is == null here
render frag

by

Doc doc = GET document
Element frag = doc.xpath(path/to/fragment)
// frag.getParent() is != null here
render frag

Another alternative that would involve some redesign is what I proposed earlier in the context of the SWiM/JOMDoc integration: that TNTBase maintains a global collection of all notation definitions found in all documents, and that we add some interface to JOMDoc that allows for finding out whether a particular notation definition is applicable when rendering the current fragment. (Suppose the current fragment F contains a symbol S whose theory T we forgot to import. The global collection contains the notation definition for S, but it may not be used when rendering F.)

  Changed 3 years ago by dmisev

you're right, this is not thread-safe. One possible solution I just came up with

public class TheoryElement extends nu.xom.Element {
    
    private String theoryName;

    public TheoryElement(Element element) {
        super(element);
    }

    public String getTheoryName() {
        return theoryName;
    }

    public void setTheoryName(String theoryName) {
        this.theoryName = theoryName;
    }

}

You pass this TheoryElement? to the renderElement (setting the theoryName appropriately), instead of passing an ordinary Element. What do you think?

  Changed 3 years ago by vzholudev

Looks ok. But setting theory name could be incorporated into the constructor.

Also one possible hack would be to use static ThreadLocal?<String> theoryName field. But I liked your solution more.

That reminds me that Builder class is not thread safe either. In my TNTBase code I have something like this:

    private static ThreadLocal<Builder> builder = new ThreadLocal<Builder>() {
        @Override
        protected Builder initialValue() {
            XMLReader reader = null;
            try {
                reader = XMLReaderFactory.createXMLReader();
            } catch (SAXException e) {
                throw new IllegalStateException("Cannot create XMLReader object");
            }
            reader.setEntityResolver(new EntityResolver() {
                public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException {
                    return new InputSource(new StringReader(""));
                }
            });
            return new Builder(reader, false);
        }
    };

    private Builder getBuilder() {
        return builder.get();
    }

  Changed 3 years ago by clange

That sounds all good to me what you are discussing here. Thanks for making me aware of the ThreadLocal? class, I didn't know it before. Let me suggest to create a new ticket, scheduled for the next milestone: check the whole JOMDoc code for thread-safety, which means:

  • either make things thread-safe
  • or document that they are not thread-safe and how to make them thread-safe (so that the application that uses JOMDoc can decide; compare Java's non-thread-safe collection classes)

  Changed 3 years ago by dmisev

Please check whether it's working now by using the DetachedElementInTheory? class for rendering a fragment.

  Changed 3 years ago by vzholudev

Do you have a code snippet to look at? I'm afraid I could potentially misuse your feature.

  Changed 3 years ago by dmisev

From DTest.testFragmentRendering()

Element fragment = ...;
fragment.detach();        
DetachedElementInTheory detached = new DetachedElementInTheory(fragment, "t1",inputDoc.getBaseURI());
Element res = r.renderElement(detached);

  Changed 3 years ago by dmisev

sorr there was a mistake, this should be correct:

Element fragment = ...;
fragment.detach();        
DetachedElementInTheory detached = new DetachedElementInTheory(fragment, inputDoc.getBaseURI(), "t1");
Element res = r.renderElement(detached);

  Changed 3 years ago by vzholudev

Just a small comment by now: Why don't you detach a 'fragment' inside a constructor?

  Changed 3 years ago by dmisev

Well I assume the element is already detached, if you wrap it in this. But ok I'll detach it in the constructor, just for any case.

  Changed 3 years ago by dmisev

ah ok, the detaching is not needed at all - when you create the DetachedElementInTheory? it creates a copy of your original element, which is automatically detached.

  Changed 2 years ago by dmisev

  • status changed from reopened to closed
  • resolution set to fixed

I suppose this can be closed.

  Changed 2 years ago by vzholudev

  • status changed from closed to reopened
  • resolution fixed deleted

Now I updated to the newest JOMDoc, adapted my code that more or less boils down to renaming class names. Now rendering of a fragment doesn't work. Any ideas why it could happen?

  Changed 2 years ago by dmisev

  • version changed from v0.1.3 to v0.1.4
  • milestone changed from Release v0.1.3 to Release v0.1.4

Maybe you have done something different? Because the test ImportsAwareTest.testFragmentRendering() still passes. I'll try with a more complex test and see how it works.

  Changed 2 years ago by vzholudev

I didn't change any code except renaming classes and changing packages. Please, let me know how will testing with a more complex example go. Thanks

  Changed 2 years ago by dmisev

  • status changed from reopened to closed
  • resolution set to fixed

Ok the problem was in the ParallelRenderer?. I didn't have any tests with parallel rendering so that's why I haven't noticed the bug. I'm not sure how it worked before :) It should be fixed now.

Note: See TracTickets for help on using tickets.