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

MAD-NG Features and Compatibility Modes #135

Draft
wants to merge 370 commits into
base: master
Choose a base branch
from
Draft

MAD-NG Features and Compatibility Modes #135

wants to merge 370 commits into from

Conversation

fsoubelet
Copy link
Member

@fsoubelet fsoubelet commented Oct 15, 2024

MAD-NG Compatibility

This PR close #104 and closes #105. I will make a new issue for some exotic caveats (see caveat subsection below).

While users relying on default parameters will not be affected, I label this as a major version release as there is a substantial amount of behavioural changes.

Main Changes

Full details relevant to the users can be found in the Changelog, details are given in sections below. Main items are:

  • Handling of boolean and complex headers values (MAD-NG feature).
  • Handling of bolean-type and complex-type columns (MAD-NG feature).
  • Compatibility modes for dataframe validation (details below).

Some changes and fixes also show up:

  • TfsDataFrame validation is now skipped on reading by default.
  • TfsDataFrame validation is by default done in MAD-X compatibility mode before writing.
  • Writing a dataframe with no headers (not empty headers) - e.g. a pandas.DataFrame - now works correctly.

MAD-NG Features

Changes have been made to support boolean and complex values, both in headers and columns.
Special care is taken for the parsing of complex values given that the MAD-NG representation uses i for the complex part while Python uses j.

I am adding comments on the PR to help review.

Caveat

Some of the more exotic features that can happen from MAD-NG are not supported, as discussed. These include for instance the overloaded ranges (i.e. 1..7) and for these the user should use pymadng and do conversions in memory. I will create an issue on the repo to be tagged "wontfix" or similar.

Should a user encounter this, then an UnknownTypeIdentifierError is raised (see maintenance changes). I am open to directing them towards pymadng in the error message.

Validation

The validation has been reworked and offers compatibility guarantees with MAD-X or MAD-NG. A valid mode should be provided to the tfs.frame.validate function, which accepts MADX / MAD-X / MADNG/MAD-NG` (case-insensitive).

Validation can still be skipped when reading or writing by providing None.

Details on the applied rules are given in the documentation so I won't duplicate them here. I'll point it out as PR comments.

Added Tests

I have added an entirely new module for all tests related to validation.
I have also added a bunch of reader and writer tests to cove the new (MAD-NG) features.
I have added new TFS files in the inputs for each of these features, and I have added a compressed version (for each supported format) of a file containing them all to the relevant folder.

Added Documentation

Docstrings have been updated everywhere relevant.

The documentation now holds a dedicated page detailing the TFS format (this closes #105).
There is also a dedicated page about validation and compatibility modes (this closes #104).

I heavily encourage downloading the built documentation from CI and having a look locally.

Maintenance Changes

The most substantial maintenance change is the introduction of many new errors to be raised, which are each specific to a given problem one might encounter. They all inherit from the (previously unique) TfsFormatError, so users that were catching this one will not see their code broken.

I have made a slew of maintenance changes to the documentation: fixing warnings during generation, updating deprecated configuration parameters, fixed typos, tried to make a lot of things more explicitely explained, fixed broken links etc.

There are also a bunch of maintenance changes made to the tests. Mainly, I have moved quite a few common fixtures to the conftest.py file to avoid the duplications we had through files. I have tried to separate a bit more different testing parts.

I will add PR comments to the code to make it easy to determine what is new vs what is maintenance.

@fsoubelet
Copy link
Member Author

"Scope creep is not real, it can't hurt you"

I have added the support for MAD-NG's nils as we discussed. With this, apart from the tables and ranges that it includes (we recommend going through pymadng for these), we can confidently announce this release brings support for MAD-NG.

Caveats of the nil implementations (I have added these to the documentation too):

  • When reading from file, nil values in the headers and string-dtyped columns are converted to None while those in the rest of the dataframe are cast to NaN.
  • When writing to file, None values anywhere are written as nil and NaN values are written as NaN (remember that setting a None in a numeric pandas.DataFrame column automatically casts it as NaN, this is pandas behaviour).

I have disallowed None values in MAD-X compatibility modes. Relevant tests for reading, writing, validation regarding nils are implemented.

  • Only need left a MAD-NG & pymadng release so I can add tests for MAD-NG reading our files.

@fsoubelet fsoubelet mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High Work on this! Status: Review Needed Work currently stopped, untils someone else reviews it. Type: Documentation Improvements, updates and fixes to the documentation. Type: Feature A (suggetion for a) new feature or enhancement in functionality. Type: Maintenance Improvements in the code, that are not necessarily visible in functionality. Type: Release Issue preparing for a release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: TFS definition [Feature Request]: Check for madx-compatibility
3 participants