LLMLingua icon indicating copy to clipboard operation
LLMLingua copied to clipboard

[design] Interface Design

Open mydmdm opened this issue 1 year ago • 0 comments

Purpose

Engine Interface

# Example of engine interface
  • get_ppl: Includes Perplexity (PPL) and Contrastive PPL calculation, supports KV-cache.
  • get_relevance_rank: Returns the relevance ranking between context and question.

Core Interface

  • coarse_level_compression_in_document: Compresses the document/demon

ation in a coarse level, allocating the budget and dynamic compression ratio.

  • coarse_level_compression_in_sentence: Performs coarse-level compression of sentences.
  • iterative_token_level_compression: Compresses the prompt at the token level.
  • subsequence_recover: Recovers based on the subsequence relationship.

Wrapper Interface

  • compress_prompt: Returns the compressed prompt.

mydmdm avatar Jan 12 '24 09:01 mydmdm

Seems like the -filter is relative to the current directory, and not the test directory. So, the following should work: opentf test -test-directory=/internal/command/testdata/test/multiple_files -filter=/internal/command/testdata/test/multiple_files/one.tftest.hcl

RLRabinowitz avatar Sep 04 '23 09:09 RLRabinowitz

So we have two ways to tackle that issue

  1. alter --help that explains filter only filter from the current directory
  2. change the filter flag behavior to filter the files can be read by using --test-directory

WDYT? any other ideas?

eranelbaz avatar Oct 05 '23 12:10 eranelbaz

My 2c. but given the commentary under the test-directory help text flag, I'd say the behavior is correct if confusing:

$ ./bin/tofu test  -help

...

  -test-directory=path  Set the OpenTofu test directory, defaults to "tests". When set, the
                        test command will search for test files [[[[in the current directory and]]]]
                        in the one specified by the flag.

([[[[emphasis]]]] mine)

In particular, -test-directory is not the same as e.g., a tar -C option, and really means this+that... -filter behaving as it does thus allows disambiguating $PWD/file vs $TESTDIR/file, seemingly following the same ordering as -test-directory.

cipherboy avatar Oct 08 '23 17:10 cipherboy

@cipherboy I didn't manage to follow, but the issue isn't the -test-directory but the -filter which filter only from the test directory

eranelbaz avatar Oct 09 '23 09:10 eranelbaz

Just an experience report on current usage: I looked back into my scripts, and I used -filter with the full path to the exact test I was currently executing, without using -test-directory, so it makes sense to me as you may want, say from an IDE, test a subset of tests. I also absolutely don't remember finding this complicated to figure out, in the absence of -test-directory that I haven't remembered ever using or discovering, but this may be due to the documentation at the time being non-existent.

serialseb avatar Oct 09 '23 11:10 serialseb

@eranelbaz Sorry, let me try rephrasing that: -test-directory is not the same as the tar --directory option, wherein tar will change to the given directory and be relative to that directory. Instead, it seems from tofu test -help that -test-directory is an additive directory: search for tests in $PWD and this new test directory specified on the CLI. It seems like it is more like appending to $PATH: add another place to search for tests, like a shell would for binaries. (If this is wrong and the help text is wrong, perhaps that should also be addressed -- but that's what my emphasized part above -- from the tofu test -help output seems to say).

So in the context of what -test-directory does, -filter thus makes (a little more) sense: it isn't relative to -test-directory because -test-directory does not do a chdir(2) and does not change tofu's view of $PWD. Instead, since -test-directory appends another directory to search, -filter is still relative to the current directory. And, you'd need to use a path describing -test-directory relative/absolute to $PWD to filter, as @RLRabinowitz showed in their example.

Supposed you had this directory structure:
/a.tf
/a.tftest.hcl
/tests/extended.tftest.hcl
/module/b.tf
/module/b.tftest.hcl
/empty

My understanding from the help text being worded the way it is, is that, if you were to execute a bare:

$ cd / && tofu test

you'd end up running a.tftest.hcl AND extended.tftest.hcl because $PWD is searched and tests is the implicit default value for -test-directory.

If you instead do:

$ cd /empty && tofu test

you'd get no tests run (no tests subdirectory and no .tftest.hcl files).

And if you were to do:

$ cd /module && tofu test

You'd just get b.tftest.hcl run.

To run just b+a's basic tests, you could do:

$ cd /module && tofu test -test-directory=/

which I think would run a.tftest.hcl and b.tftest.hcl but not extended.tftest.hcl if that makes sense.

Note that to only execute a.tftest.hcl, you'd have to do the filter relative to $PWD, so perhaps either:

$ cd /module && tofu test -test-directory=/ -filter=/a.tftest.hcl
--or--
$ cd /module && tofu test -test-directory=/ -filter=../a.tftest.hcl

so that b.tftest.hcl does not run.

i.e., without -test-directory specified, you get something similar to TF_TEST_MODULE_PATH=$PWD:tests and with it set to some value -test-directory=$TESTDIR, you get TF_TEST_MODULE_PATH=$PWD:$TESTDIR


So, I'd definitely opt for consistency, perhaps rephrasing the given options:

  1. Align and clarify -filter's and -test-directory's help text, so this behavior is less confusing. :-)
  2. Change the behavior both of -test-directory and -filter to be something different (but, keeping it so that they still align) -- probably dropping the $PWD behavior that seems to be tacked on and make it more like tar -C.

My 2c. :-)

cipherboy avatar Oct 09 '23 11:10 cipherboy

I think @cipherboy's comment makes sense here. We'll just check if the docs are clear enough as of now, and not change the behaviour.

cube2222 avatar Jan 18 '24 13:01 cube2222

[...] Instead, it seems from tofu test -help that -test-directory is an additive directory: search for tests in $PWD and this new test directory specified on the CLI. It seems like it is more like appending to $PATH: add another place to search for tests, like a shell would for binaries. (If this is wrong and the help text is wrong, perhaps that should also be addressed -- but that's what my emphasized part above -- from the tofu test -help output seems to say).

If the behavior were additive, aka search the current directory and the given test-directory, why does the filter OP ran not find the one.tftest.hcl test?

I have set up the following directory structure:

opentofu/
  some-other-test.tftest.hcl
  test-directory/
    one.tftest.hcl

Running tofu test results in some-other-test.tftest.hcl being run (which makes sense):

~/Documents/coding-projects/opentofu$ tofu test
some-other-test.tftest.hcl... pass

Success! 0 passed, 0 failed.

Running tofu test -test-directory=test-directory results in both tests being run (still in the realm of the expected):

~/Documents/coding-projects/opentofu$ tofu test -test-directory=test-directory
some-other-test.tftest.hcl... pass
test-directory/one.tftest.hcl... pass

Success! 0 passed, 0 failed.

Running tofu test -test-directory=test-directory -filter=one.tftest.hcl results in neither test being run (this is the odd part):

~/Documents/coding-projects/opentofu$ tofu test -test-directory=test-directory -filter=one.tftest.hcl

Success! 0 passed, 0 failed.

You can get just one.tftest.hcl to run if you specifically use test-directory/one.tftest.hcl in the filter:

~/Documents/coding-projects/opentofu$ tofu test -test-directory=test-directory -filter=test-directory/one.tftest.hcl
test-directory/one.tftest.hcl... pass

Success! 0 passed, 0 failed.

I would say that this is unintuitive.

If you are not looking to change behavior, I would suggest adding a note to the -filter help text and the documentation. Something along the lines of:

~/Documents/coding-projects/opentofu$ tofu test -h
Usage: tofu [global options] test [options]

  Executes automated integration tests against the current OpenTofu 
  configuration.

  [...]

Options:

  -filter=testfile      If specified, OpenTofu will only execute the test files
                        specified by this flag. You can use this option multiple
                        times to execute more than one test file.
                        Note: If used alongside -test-directory=path,
                        any filters for test files in the test-directory must be
                        prepended by the test-directory.

[...]

  -test-directory=path  Set the OpenTofu test directory, defaults to "tests". When set, the
                        test command will search for test files in the current directory and
                        in the one specified by the flag.

[...]

ccrpc-fjunge avatar Jan 18 '24 16:01 ccrpc-fjunge

@ccrpc-fjunge wrote:

If the behavior were additive, aka search the current directory and the given test-directory, why does the filter OP ran not find the one.tftest.hcl test?

Because -filter isn't relative to -test-directory (or just operating on bare file names without directory components), it is relative to $PWD of the CLI execution environment. :-)

See my expanded note Supposed you had this directory structure: in my above comment; it aligns with your comment findings.

I agree, it is not intuitive, but once understanding the reason ($PWD does not change and it does not behave like tar's --directory option), it does make sense that this is the behavior.

cipherboy avatar Jan 18 '24 16:01 cipherboy

Yes, knowing how the program actually works: its behavior makes sense. I am in no way attempting to contradict your findings, and your explanation of "this is why it works the way it does" is spot on.

That said when it comes to documentation, you are attempting to communicate how the program will work without having people have to write test cases and pull it apart themselves. Neither should the explanation of the behavior be only available in (very well written) Github comments on an issue on the project's repository.

Mind you half of my searches when attempting to debug something do end up leading to Github comments by people similarly confused or submitting a bug report, and that often resolves the issue, but that is really not the ideal scenario.

Overall, in the current documentation the need for the paths is not clarified.

Now this issue is mitigated somewhat in that if you have a tests directory (which is what the default test-directory is called) and you are using the -filter option you also have to specify the tests/ part of the path. i.e. If you have a file in the tests folder named yet-another-test.tftest.hcl, you need to use

tofu test -filter=tests/yet-another-test.tftest.hcl

The following will not work:

tofu test -filter=yet-another-test.tftest.hcl

This matches the behavior of a defined -test-directory=, which is good! Consistency!

In fact with that consistency, if you could document somewhere the need for paths for any test not in the folder the CLI is being executed in that would cover both the case of the default tests directory as well as someone setting the -test-directory= option.

ccrpc-fjunge avatar Jan 18 '24 17:01 ccrpc-fjunge