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

[API] Better handling of execution modes #797

Merged
merged 22 commits into from
May 21, 2024
Merged

Conversation

JasonMarechal25
Copy link
Contributor

@JasonMarechal25 JasonMarechal25 commented Apr 26, 2024

  • Properly distinguish between archive, file and study mode
  • study mode (using antares lib) not implemented yet
  • Code cleanup
  • Refactor how to handle mandatory but exclusive parameters. Hopefully make it easier to add new one in the future

@JasonMarechal25 JasonMarechal25 changed the base branch from develop to feature/refacto_test_lpnamer April 26, 2024 12:06
@JasonMarechal25 JasonMarechal25 requested review from tbittar and a team April 26, 2024 12:40
Base automatically changed from feature/refacto_test_lpnamer to feature/clean_up May 14, 2024 09:00
Base automatically changed from feature/clean_up to develop May 14, 2024 13:28
src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/main/ProblemGeneration.cpp Outdated Show resolved Hide resolved

CreateDirectories(deduced_xpansion_output_dir);
CreateDirectories(xpansion_output_dir); // Ca ou -Xpansion ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the Python part (or any other part of the code) rely on the -Xpansion suffix ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but the conditions are foggy

src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
@@ -187,6 +223,8 @@ void ProblemGeneration::RunProblemGeneration(
antares_version < first_version_without_variables_files;
(*logger)(LogUtils::LOGLEVEL::INFO)
<< "rename problems: " << std::boolalpha << rename_problems << std::endl;


auto files_mapper = FilesMapper(antares_archive_path, xpansion_output_dir);
auto mpsList = files_mapper.MpsAndVariablesFilesVect();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do this in ANTARES_API mode ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming? Probably not. Adding a todo in #753

Copy link
Contributor

Choose a reason for hiding this comment

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

No I meant creating the mpsList object (it seems unused in ANTARES_API mode) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes in the final implementation they're not used for API mode.

src/cpp/lpnamer/main/ProblemGenerationExeOptions.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/main/ProblemGenerationExeOptions.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/main/ProblemGenerationExeOptions.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/input_reader/LpFilesExtractor.cpp Outdated Show resolved Hide resolved
src/cpp/lpnamer/input_reader/LpFilesExtractor.h Outdated Show resolved Hide resolved
Copy link

@JasonMarechal25 JasonMarechal25 merged commit 50bbe69 into develop May 21, 2024
9 checks passed
@JasonMarechal25 JasonMarechal25 deleted the feature/modes branch May 21, 2024 08:05
@JasonMarechal25 JasonMarechal25 mentioned this pull request Aug 1, 2024
18 tasks
JasonMarechal25 added a commit that referenced this pull request Sep 13, 2024
Introduce "study mode". A way to run xpansion and problem generation
with a study folder as parameter. In this case Antares will not be run
as a stand-alone but as a library by problem generation.

- New ADRs
- Add a "new" data set for lpnamer E2E tests: SmallTestFiveCandidates, a
copy of the example provided by Xpansion.
- Refactor test_lpnamerEndToEnd.py : when possible prefer the use
multiple @parametrize instead of writing the combinations in a list.
- Refactor test_lpnamerEndToEnd.py : copy the test data in a temporary
folder and works there. Prevent creating outputs in source code.
- Add a test case for Study mode in test_lpnamerEndToEnd.py
- Add a new E2E test: short memory
- Refactor ProblemGenerationExeOptionsTest to test several combinations
of mutually exclusive parameters
- In SolverFactory.cpp call Init() on solver objects before returning
them. Prevent temporal coupling for Xpress solver where a call to init()
was mandatory.
- Add Antares as a build dependency


Missing pieces (every thing is open to discussion):

- [ ] Handle Antares error #888 
- [x] Update general data ini
- [ ] Revert general data ini #889
- [x] Write ADR
- [ ] Update sequence diagram #890
- [ ] Maybe update C4 #890
- [x] Use enum mode in LPFileExtractor #797 
- [ ] find/define a way to log before lp dir exist (for antares error
for exemple) #888
- [ ] Handle variables.txt directly at Lps level
(SignificationMetierDesVariables)
- [ ] Factorise variables name and "calculate" time step with week
number on xpansion side (A faire dans Antares bien et vite)
- [x] Cleanup python to only support memory mode (See ADR, we keep some
support for now)
- [x] antares named problèmes force mps writing. Peut etre pas besoin de
l'option named sachant que les variables sont nommées dans la structure
LP
- [ ] Mettre à jour les docs: notamment comment build, dépendances, etc
#893
- [ ] Investigate to use full run executable with --study option and use
fullrun driver #892
- [x] Handle PBGen step with memory
- [ ] Add entry in multisolver to rename a range of col/var
#753 (comment)
#891
- [ ] Run Antares in parallele mode

Following other PR:

- [ ] Don't overly rename problem in API mode

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: abdoulbari zakir <[email protected]>
Co-authored-by: tbittar <[email protected]>
Co-authored-by: Thomas Bittar <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants