Refactor cmontecarlo tests for exercising parametrization. #530
Conversation
d78b547 to
0a0c9a1
Compare
| tests = CDLL(test_path) | ||
|
|
||
| cmontecarlo_filepath = os.path.join(path[0], 'montecarlo', 'montecarlo.so') | ||
| cmontecarlo_methods = CDLL(cmontecarlo_filepath) |
There was a problem hiding this comment.
The loading of the CDLL is something specific to the tests, this shouldn't be in struct.py, only the definition of RPacket and StorageModel
There was a problem hiding this comment.
Okay, I will move it to test_cmontecarlo.py
d47d35a to
b01ce9d
Compare
|
Rebasing upon merge of #514 caused failing of tests. Tests have to be rectified. |
| ({'virtual_packet': 1}, {'cont_status': 0}, {'chi_cont': 6.652486e-16, 'd_cont': 1e+99}), | ||
| ({'virtual_packet': 1}, {'cont_status': 1}, {'chi_cont': 6.652486e-16, 'd_cont': 1e+99})] | ||
| ) | ||
| def test_compute_distance2continuum(packet_params, model_params, expected_params, packet, model): |
There was a problem hiding this comment.
Thanks for writing a test for that method. Currently continuum interaction is a feature in development. I'd say it is safe for now to only test cont_status=0. @chvogl Is there any reason to test cont_status=1 in the current master?
There was a problem hiding this comment.
Okay, I will remove the two test cases having cont_status=1
There was a problem hiding this comment.
@yeganer There is no reason to test cont_status=1 in the current master.
d705e2e to
17a06a8
Compare
| # TODO: Add scientifically sound test cases. | ||
| [({'virtual_packet': 1, 'tau_event': 2.9e13, 'last_line': 0}, | ||
| {'tau_event': 2.9e13, 'next_line_id': 2}), | ||
|
|
There was a problem hiding this comment.
@yeganer I am just dealing with numbers.
68bb193 to
0c1b2a4
Compare
- C structs in rpacket.h, storage.h and rk_mt.h have been replicated in struct.py as per the design technique while using C types. These structs will now be used to pass in wrapped C methods.
- Three fixtures, each for rpacket, storage model and mt_state were added in test_cmontecarlo.py - These fixtures have the same declaration as it was in init methods of corresponding C tests.
- These methods were refactored as per design
procedure of test_distance2boundary:
- test_distance2line
- test_distance2continuum
|
@yeganer - how is this PR review coming along? |
| 'status': 0, | ||
| 'id': 0 | ||
| } | ||
| return RPacket(**packet_default) |
There was a problem hiding this comment.
I'd prefer writing RPacket({ ...}) and removing the variable packet_defaul. That would look cleaner to me.
|
@karandesai-96 nice work on adding some documentation to the beginning of the file. |
| doppler_factor = 0.9998556693818854 | ||
| tests.test_rpacket_doppler_factor.restype = c_double | ||
| assert_almost_equal(tests.test_rpacket_doppler_factor(), | ||
| doppler_factor) | ||
|
|
||
| @pytest.mark.skipif(True, reason='Bad test design') |
There was a problem hiding this comment.
I'd move this function blow the new tests. That way we have new & old nicely grouped together.
There was a problem hiding this comment.
I was talking about test_move_packet. Sorry for not being clear.
There was a problem hiding this comment.
Ah, I misunderstood it as "Move test_rpacket_doppler_factor below to group old and new tests nicely in diffs" 😄
- Done.
- Declare kwargs in fixtures directly instead of passing a dict as kwargs. - Move test_move_packet to group new/ old tests.
Continue cmontecarlo tests refactor. Extend #530.
Current design procedure of tests:
ctypeslibrary.This PR refactors the current tests such that: