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

io.compass - Use last revision by default #55

Open
hetsko opened this issue Oct 27, 2022 · 2 comments
Open

io.compass - Use last revision by default #55

hetsko opened this issue Oct 27, 2022 · 2 comments

Comments

@hetsko
Copy link

hetsko commented Oct 27, 2022

Function pleque.io.compass.cdb() uses revision=1 as a default parameter value. I would propose a change to revision=-1, i.e. the most recent revision.

Arguments:

  • On the one hand, the original behaviour enforces some degree of reproducibility of the returned value on subsequent calls. On the other hand, there are cases where the revision=1 data in CDB are broken (cause exception when you attempt to load them) but a fixed version of the data are available in revision=2. As a result, the original behaviour ensures that that particular discharge will always throw error, possibly misleading the user into thinking that no "fixed data" exist.
  • A second argument for the change is the fact that the pyCDB client loads the most recent revision by default. A user then might logically expect the same behaviour from pleque as well, since this feels as the defining property/philosophy of the CDB database itself (i.e. "last revision is always/eventually the best revision which everyone should use").
@kripnerl
Copy link
Owner

The main argument for keeping its 1 experimental database (CDB) is that the first revision marks the EFIT run's default configuration. It means that the database-wide search is less susceptible to mistakes. This behaviour is well documented in the cdb docstring. Due to the lack of using variants when using EFIT, later revisions may return inconsistent data.

A possible solution to this issue may be trying to initiate cdb discharge from the first revision and incrementally increase the revision while issuing a warning.

The philosophy of pyCDB is indeed correct. However, it is not aligned with the former usage of CDB and not using variants but using new revisions instead.

Therefore, I am inclined to keep the current behaviour.

@hetsko
Copy link
Author

hetsko commented Mar 20, 2023

Okay.

Then, the revision incrementing sounds interesting, although I would be more inclined to let the user handle the error state themselves (you can be never sure, why the error appeared, it could be just a temporary loss of connection to the database instead of broken revision). Meaning that it would be great to inject a more informative message and/or suggestion to try different revision when an error occurs (and maybe even check that there are newer revisions).

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

No branches or pull requests

2 participants