Skip to content

Commit

Permalink
SimultaneousState list saving was dropping FOVs (#97)
Browse files Browse the repository at this point in the history
* SimultaneousState saving vec conversion bug

SimultaneousState.save_list was not working correctly due to a conversion order of operations issue.
The conversion between python and rust for list of SimultaneousStates was unintentionally dropping the FOV information.
This was leading to errors when attempting to save simulation results which included this information.

This was fixed by changing the order of operations to try SimState conversions before list conversions.
Error messages were edited to increase the clarity of the bug.

* changelog
  • Loading branch information
dahlend authored Aug 18, 2024
1 parent 0dedca8 commit 15edf78
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Field of View checks for states was optimized for multi-core processing on millions
of objects, leading to huge speed gains for large queries.
- Fixed a bug where saving lists of SimultaneousStates had a bug where field of view
information was not being saved correctly.

## [0.2.5] - 2024 - 8 - 12

Expand Down
17 changes: 12 additions & 5 deletions src/neospy/rust/simult_states.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use neospy_core::errors::Error;
use neospy_core::io::FileIO;
use neospy_core::simult_states;
use pyo3::exceptions;
Expand Down Expand Up @@ -25,12 +26,18 @@ impl From<simult_states::SimultaneousStates> for PySimultaneousStates {

impl<'py> FromPyObject<'py> for PySimultaneousStates {
fn extract_bound(ob: &Bound<'py, PyAny>) -> PyResult<Self> {
if let Ok(states) = ob.extract::<Vec<PyState>>() {
return PySimultaneousStates::new(states, None);
match ob.downcast_exact::<PySimultaneousStates>() {
Ok(downcast) => Ok(PySimultaneousStates(downcast.get().0.clone())),
Err(_) => {
if let Ok(states) = ob.extract::<Vec<PyState>>() {
PySimultaneousStates::new(states, None)
} else {
Err(Error::ValueError(
"Input could not be converted to a SimultaneousStates".into(),
))?
}
}
}
Ok(PySimultaneousStates(
ob.downcast_exact::<PySimultaneousStates>()?.get().0.clone(),
))
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/neospy_core/src/simult_states.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ impl SimultaneousStates {
{
return Err(Error::ValueError("Center IDs do not match expected".into()));
};
if fov.is_none() && !states.iter_mut().all(|state: &mut State| state.jd == jd) {
return Err(Error::ValueError("Epoch JDs do not match expected".into()));
if fov.is_none() && states.iter_mut().any(|state: &mut State| state.jd != jd) {
return Err(Error::ValueError(
"Epoch JDs do not match expected, this is only allowed if there is an associated FOV."
.into(),
));
};
Ok(SimultaneousStates {
states,
Expand Down

0 comments on commit 15edf78

Please sign in to comment.