radiasoft / rslaser_old

Integrated modeling of CPA crystal-based laser amplifiers
Apache License 2.0
1 stars 1 forks source link

Fix #127 change slice params #131

Closed gurhar1133 closed 1 year ago

gurhar1133 commented 1 year ago

The laserpulse and laserpulseslice now take a non-heirarchical params as input.

I removed references to envelopes within the LaserPulse and LaserPulseSlice.

@bruhwiler by removing references to the envelope did you mean you wanted me to delete the class itself? Do you think the new params structure works?

bruhwiler commented 1 year ago

There are test failures in pulse_test.py and wavefront_sensor_test.py However, these failures are understood and are being addressed elsewhere.

gurhar1133 commented 1 year ago

There are test failures in pulse_test.py and wavefront_sensor_test.py However, these failures are understood and are being addressed elsewhere.

Which branch has failures? I am not finding any on this branch or master.

bruhwiler commented 1 year ago

Regarding test failures, once I did a new 'pip install -e .' in my Sirepo-Jupyter environment, all tests are passing.

Sorry for the confusion.

bruhwiler commented 1 year ago

Please don't remove the envelope class.

bruhwiler commented 1 year ago

Based on our discussion, please move all parameters into the _LASER_PULSE_DEFAULTS and eliminate the concept of a _LASER_PULSE_SLICE_DEFAULTS

The LaserPulseSlice constructor is expecting to receive the LaserPulse params object anyway.

The fundamental confusion was having a LaserPulseSlice params object embedded in the LaserPulse params object.

gurhar1133 commented 1 year ago

@bruhwiler made the changes we discussed. Let me know your thoughts if more needs to be changed 👍

bruhwiler commented 1 year ago

In restoring the logic for correct calculation of field energy in a laser pulse slice, I inadvertently restored 3 instances of .slice_params. which broke a couple of tests.

I corrected that error with my most recent commit, but I still have one test failing: tests/wavefront_sensor_test.py::test_propagation01 FAILED

bruhwiler commented 1 year ago

Regarding the question about '''-self._pulse_pos + self.dist_waist''' I don't think I broke anything there.

bruhwiler commented 1 year ago

I overwrote the test data, so by definition the problematic test is now passing. I think this is OK, but we need to review that test data again soon.