search

rivetTDA / rivet

RIVET is a tool for Topological Data Analysis, in particular two-parameter persistent homology.
GNU General Public License v3.0
73 stars 24 forks source link

Input #145

Closed hackerde closed 3 years ago

hackerde commented 5 years ago

Related to Issue #138

Introduces a new and user friendly format for creating input files for RIVET.

Features:

All the old data files have been updated to adopt the new format and documentation has been updated to reflect the changes made.

mlesnick commented 5 years ago

Looks like good progress, thank you.

What is the expected behavior if no key-value pairs are given?
For points, if there is no top matter, I think it could be ambiguous whether one is looking at 3-D data with a function, or 4-D data with no function.

Also, is this intended to be a replacement for the "points" data type? Or do we intend to keep the "points" data type as is?

hackerde commented 5 years ago

We discussed this and came to the conclusion that if the FUNCTION key is not specified, then it means there is no function and hence it would be interpreted as 4-D data.

For now, the changes are implemented as addition. But I believe we might consider this to be a replacement after some discussion.

mlwright84 commented 5 years ago

This looks very good, but I want to run some more tests.

While discussing with Mike last week, he suggested that we remove dashes from within the flags. That is, the flag x-label should be changed to xlabel, and similarly for other flags. I think even hom_degree could be changed to homdegree.

To make our example files more readable, I think we should include a comment, such as #data, before the data section of the file. I don't think we should require a flag, such as --data, so that RIVET will support CSV files that have only data and no flags.

It's also important that we pair these code updates with updates to the documentation. I understand that such updates are in progress.

xoltar commented 5 years ago

Hi Folks, also please keep in mind that the python API works by invoking rivet_console, so if we change the flags, we should make a new release (i.e. a version number and a git tag), and we'll need to update the python API as well. I'm all in favor of simplifying flags, but just wanted to make sure the python code gets updated appropriately as well.

Thanks, Bryn

On Mon, Jul 15, 2019 at 11:54 AM Matthew Wright notifications@github.com wrote:

This looks very good, but I want to run some more tests.

While discussing with Mike last week, he suggested that we remove dashes from within the flags. That is, the flag x-label should be changed to xlabel, and similarly for other flags. I think even hom_degree could be changed to homdegree.

To make our example files more readable, I think we should include a comment, such as #data, before the data section of the file. I don't think we should require a flag, such as --data, so that RIVET will support CSV files that have only data and no flags.

It's also important that we pair these code updates with updates to the documentation. I understand that such updates are in progress.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rivetTDA/rivet/pull/145?email_source=notifications&email_token=AAJLLXCWCT3UUXIWEHWQLZTP7TBUTA5CNFSM4H22WN7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ6UG7I#issuecomment-511525757, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJLLXD4XMOLH52WC5I7GZLP7TBUTANCNFSM4H22WN7A .

mlwright84 commented 5 years ago

Bryn, thanks for the reminder about the Python API. I think that all of the older command-line flags are still supported. We have added many new flags, and also changed the input formats so that parameters can be specified in the input file with the same syntax used for command-line flags.

When this pull request is merged, we will give it a new version number and make sure that the Python code is updated as necessary.

hackerde commented 5 years ago

While discussing with Mike last week, he suggested that we remove dashes from within the flags. That is, the flag x-label should be changed to xlabel, and similarly for other flags.

The intention of the dashes within flags was to keep it similar to command line flags which have dashes in the middle if it is multiple words. However, I can easily remove them if that is the design we are going for. Maybe they are unnecessary for our purposes.

I think even hom_degree could be changed to homdegree.

hom_degree is a variable name and not a flag. The flag is --homology. From what I've seen in RIVET, variables (and function names) have an underscore separating multiple words and that was the design I was following. But as always, I can get rid of the underscore if that is what we want.

mlwright84 commented 5 years ago

While discussing with Mike last week, he suggested that we remove dashes from within the flags. That is, the flag x-label should be changed to xlabel, and similarly for other flags.

The intention of the dashes within flags was to keep it similar to command line flags which have dashes in the middle if it is multiple words. However, I can easily remove them if that is the design we are going for. Maybe they are unnecessary for our purposes.

I think that the dash should be removed from x-label, y-label, x-reverse, and y-reverse, both for command line flags and for flags in input files. Removing the dash will make these flags easier to type, and will add consistency with the flags xbins and ybins.

I think even hom_degree could be changed to homdegree.

hom_degree is a variable name and not a flag. The flag is --homology. From what I've seen in RIVET, variables (and function names) have an underscore separating multiple words and that was the design I was following. But as always, I can get rid of the underscore if that is what we want.

Good point. Let's not change hom_degree.