Skip to content

Conversation

@EZoni
Copy link
Member

@EZoni EZoni commented Feb 12, 2025

This PR introduces additional infrastructure to enhance development:

  • Unit Tests: Implemented using pytest to ensure code reliability.
  • CI Workflow: Automates the execution of unit tests on every push and pull request.
    The primary goal is to translate the original demo code into comprehensive unit tests, which will be expanded as development progresses.

During the process, I identified some issues in the original demo, likely due to changes in pydantic since the initial implementation. To address these, I am re-implementing the original elements one by one, ensuring each part is thoroughly tested according to the original demo.

I have also cleaned the .gitignore file to make it as minimal as necessary. Additional entries will be added as needed.

To-Do:

  • Fix issue with GitHub Actions described in Missing settings in forked repository #4
  • Implement unit tests to cover read/write operations in different formats, based on the original demo:
    • YAML
    • JSON
  • Implement drift (first draft)
  • Implement quadrupole (first draft)
  • Implement FODO example (first draft)

Follow-up PRs:

  • I/O in TOML format
  • Improve unit tests modularity to reduce code duplication

Examples

YAML

Example of YAML formatting:

element: Line
line:
- element: BaseElement
  name: element1
- element: ThickElement
  length: 2.0
  name: element2

Underlying (pseudo-)code:

# Serialize the Line object to YAML
yaml_data = yaml.dump(line.model_dump(), default_flow_style=False)
print(f"\n{yaml_data}")
# Write the YAML data to a file
with open("line.yaml", "w") as file:
    file.write(yaml_data)
# Read the YAML data from the file
with open("line.yaml", "r") as file:
    yaml_data = yaml.safe_load(file)

JSON

Example of JSON formatting:

{
  "element": "Line",
  "line": [
    {
      "element": "BaseElement",
      "name": "element1"
    },
    {
      "element": "ThickElement",
      "length": 2.0,
      "name": "element2"
    }
  ]
}

Underlying (pseudo-)code:

# Serialize the Line object to JSON
json_data = json.dumps(line.model_dump(), sort_keys=True, indent=2)
print(f"\n{json_data}")
# Write the JSON data to a file
with open("line.json", "w") as file:
    file.write(json_data)
# Read the JSON data from the file
with open("line.json", "r") as file:
    json_data = json.loads(file.read())

Notes

@EZoni EZoni force-pushed the start_pals_schema branch from 0de8f72 to b764d2f Compare February 12, 2025 23:55
@ax3l

This comment was marked as resolved.

@EZoni EZoni force-pushed the start_pals_schema branch from b2ca930 to 3390169 Compare February 18, 2025 19:05
@ax3l ax3l self-requested a review March 11, 2025 17:18
@@ -1,4 +1,4 @@
name: pals
name: pals-python
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The -python suffixes added here are not really needed.

From the Python viewpoint, this repo is "pals".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cemitch99

I think this original "line" concept might be what PALS calls "lattice branch". What do you think?

@EZoni EZoni requested a review from cemitch99 March 26, 2025 16:13
Copy link

@cemitch99 cemitch99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good starting point, thanks a lot. Verified that the tests in test_schema.py work (for me locally). My main comment is that some documentation on installation and running of the tests would be useful for collaborators.

@EZoni EZoni force-pushed the start_pals_schema branch from 565876e to 88618c4 Compare April 2, 2025 00:15
@EZoni
Copy link
Member Author

EZoni commented Apr 2, 2025

@cemitch99

Here's an example of how the current draft implementation of the quadrupole element works.

The code (extracted from a unit test)

# Create one drift element with custom name and length
element_name = "quadrupole_element"
element_length = 1.0
element_magnetic_multipole_Bn1 = 1.1
element_magnetic_multipole_Bn2 = 1.2
element_magnetic_multipole_Bs1 = 2.1
element_magnetic_multipole_Bs2 = 2.2
element_magnetic_multipole_tilt1 = 3.1
element_magnetic_multipole_tilt2 = 3.2
element_magnetic_multipole = MagneticMultipoleParameters(
    Bn1=element_magnetic_multipole_Bn1,
    Bs1=element_magnetic_multipole_Bs1,
    tilt1=element_magnetic_multipole_tilt1,
    Bn2=element_magnetic_multipole_Bn2,
    Bs2=element_magnetic_multipole_Bs2,
    tilt2=element_magnetic_multipole_tilt2,
)
element = QuadrupoleElement(
    name=element_name,
    Length=element_length,
    MagneticMultipoleP=element_magnetic_multipole,
)
# Serialize the Line object to YAML
yaml_data = yaml.dump(element.model_dump(), default_flow_style=False)
print(f"\n{yaml_data}")

produces the standard output

Length: 1.0
MagneticMultipoleP:
  Bn1: 1.1
  Bn2: 1.2
  Bs1: 2.1
  Bs2: 2.2
  tilt1: 3.1
  tilt2: 3.2
element: QuadrupoleElement
name: quadrupole_element

@EZoni EZoni force-pushed the start_pals_schema branch from 88618c4 to 8d86029 Compare April 2, 2025 00:48
@EZoni EZoni force-pushed the start_pals_schema branch from adc0689 to 70d3733 Compare April 12, 2025 00:04
@EZoni EZoni changed the title [WIP] Preliminary setup: unit tests, CI workflow Add Unit Tests, First Elements, Examples Apr 12, 2025
@EZoni EZoni force-pushed the start_pals_schema branch 2 times, most recently from 559cc9e to aabd7ec Compare April 24, 2025 18:57
@EZoni EZoni force-pushed the start_pals_schema branch from aabd7ec to f131afd Compare April 24, 2025 18:58
@EZoni
Copy link
Member Author

EZoni commented Apr 24, 2025

@cemitch99

I think this PR is ready for review and merge. I would proceed with further changes in follow-up PRs, since this is introducing a lot of prerequired infrastructure as well.

@EZoni EZoni requested a review from cemitch99 April 24, 2025 19:00
Copy link

@cemitch99 cemitch99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests are working. When I run the fodo.py example, I see that the YAML output is as follows:

kind: Line
line:
- kind: Drift
  length: 0.25
  name: drift1
- MagneticMultipoleP:
    Bn1: 1.0
  kind: Quadrupole
  length: 1.0
  name: quad1
- kind: Drift
  length: 0.5
  name: drift2
- MagneticMultipoleP:
    Bn1: -1.0
  kind: Quadrupole
  length: 1.0
  name: quad2
- kind: Drift
  length: 0.5
  name: drift3

The syntax is slightly different from the most recently-updated examples in the standard documentation, such as:

  line:
    - thingB           
    - Drift:            
        name: thingC
    - Quadrupole:      
        direction: -1
          ...
    - Beamline:            
        repetition: 3
          ...

The main differences are the absence of the kind keyword and the second level of indentation for parameters. It contains the same information, so I'm not sure how important this is at this stage.

@cemitch99
Copy link

It is ok with me to isolate further changes to a follow-up.

@EZoni
Copy link
Member Author

EZoni commented Apr 28, 2025

Thanks, @cemitch99.

Yes, that is the kind of things I would like to address separately because I think they are not trivial.

This YAML output

- kind: Drift
  length: 0.25
  name: drift1

reflects an object of type Drift and prints all its attributes (kind, name, and length) with the syntax attr: value and with the same indentation.

That object should be all we need, so if I want to achieve this instead

- Drift:
    length: 0.25
    name: drift1

I might need to do something custom/manual, like an extra wrapping class or something along these lines.

@EZoni EZoni merged commit ac4f4bd into pals-project:main Apr 28, 2025
3 checks passed
@EZoni EZoni self-assigned this Apr 28, 2025
@EZoni EZoni deleted the start_pals_schema branch April 28, 2025 18:38
@DavidSagan
Copy link
Member

Remember that the base documentation does not mandate the syntax of the YAML file. Just the logical structure.

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.

4 participants