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

data_storage_valid check killing simulation when storage if full #72

Closed
LorenzzoQM opened this issue Sep 27, 2023 · 4 comments · Fixed by #73
Closed

data_storage_valid check killing simulation when storage if full #72

LorenzzoQM opened this issue Sep 27, 2023 · 4 comments · Fixed by #73
Assignees
Labels
bug Something isn't working modeling Satellite and environment capabilities

Comments

@LorenzzoQM
Copy link
Contributor

Satellite data_storage_valid check killing the simulation (failed data_storage_valid check) when data buffer is equal to maximum storage capacity due to numerical rounding error (storage level slightly above storage capacity).

Version (please complete the following information):

  • Python: [3.11.5]
  • Basilisk: [2.2.1]
  • Platform: [MacOS 13.5.2]
@LorenzzoQM LorenzzoQM added bug Something isn't working triage Needs to be reviewed by a maintainer labels Sep 27, 2023
@Mark2000 Mark2000 removed the triage Needs to be reviewed by a maintainer label Sep 27, 2023
@Mark2000 Mark2000 linked a pull request Sep 27, 2023 that will close this issue
15 tasks
@Mark2000 Mark2000 reopened this Oct 17, 2023
@Mark2000
Copy link
Contributor

@LorenzzoQM reopening this since I think the behavior should be to fail if the buffer is approximately full when storageUnitValidCheck. Didn't catch that on the PR. Should be something like:

    def data_storage_valid(self) -> bool:
        """Check that the buffer has not run out of space."""
        storage_check = self.storageUnitValidCheck
        if storage_check:
            return self.storage_level < self.storageUnit.storageCapacity and not np.isclose(
                self.storage_level, self.storageUnit.storageCapacity
            )
        else:
            return True

with and not replacing what used to be or

@Mark2000 Mark2000 added the modeling Satellite and environment capabilities label Oct 31, 2023
@Mark2000
Copy link
Contributor

Mark2000 commented Dec 5, 2023

@LorenzzoQM can this issue be closed with your bsk changes to make storage an int?

@LorenzzoQM
Copy link
Contributor Author

@Mark2000 yes! This issue can be closed now that the storage unit uses int.

@LorenzzoQM
Copy link
Contributor Author

Basilisk now uses int variable type for storage, avoiding numerical rounding errors and fixing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working modeling Satellite and environment capabilities
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants