Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow no-chunking behavior for pengines. #56

Closed
wants to merge 1 commit into from

Conversation

meditans
Copy link
Contributor

The old options chunk(integer) is now optional. In the case it's not set, we fall back to a non-chunking behavior that avoids the usage of findnsols (and the effects on global variables associated with its backtracking). Initially, this was motivated by the fact that I wanted the pengine top-level (which is recursive in nature) to respect the chr store, but I also think it's a more principled thing to do.

The old options chunk(integer) is now optional. In the case it's not
set, we fall back to a non-chunking behavior that avoids the usage of
findnsols (and the effects on global variables associated with its
backtracking). Initially, this was motivated by the fact that I wanted
the pengine top-level (which is recursive in nature) to respect the chr
store, but I also think it's a more principled thing to do.
@JanWielemaker
Copy link
Member

This pull request has been mentioned on SWI-Prolog. There might be relevant details there:

https://swi-prolog.discourse.group/t/persistent-chr-store/6977/15

@meditans
Copy link
Contributor Author

In reality, I think it would have been clearer if I could have specified the chunk option as:
oneof([integer, no_chunk]), but that wasn't the semantic of oneof. If you have better suggestions here I'm eager to hear them.

@JanWielemaker
Copy link
Member

Thanks. Edited and changed somewhat. I agree that relying on a lacking chunk is not good and breaks e.g., SWISH. Of course, that can be fixed in SWISH. but as SWISH and Prolog develop independently that will lead to problems.

There are disjunctive types, so it now supports chunk=false. Hope that satisfies your requirements.

Note that I edited your commit, so it is applied but reported as a conflict.

Please close this PR if you agree.

@meditans
Copy link
Contributor Author

meditans commented Nov 23, 2023

Thank you for reviewing and merging, in the current version I'm hitting a type error when I try to use chunk=false:

Type error: integer' expected, found no_chunk' (an atom)

I'll try to look into it tomorrow and suggest a patch, as another PR. I'll close this one in the meantime, thanks again!

@meditans meditans closed this Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants