polyaxon: log_artifact naming problem
Describe the bug
Using log_artifact results in weired artifact name. Also usage is inconsistent with documentation.
To Reproduce
epoch = 1
cpt_name = 'checkpoint_epoch_' + str(epoch)
save_path = os.path.join(args.local_artifacts_path, cpt_name + '.pt')
torch.save(model, save_path)
experiment.log_artifact(save_path, name=cpt_name)
This will log it as checkpoint_epoch_1.pt. In documentation name is mentioned as optional and will be used from path param but throws error if not passed.
Things get complicated when someone tries to log file with name having multiple ‘.’ For example,
logging file.tar.gz will required us to pass name=“file.tar”
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 15 (12 by maintainers)
Commits related to this issue
- Fix log_artifact base name handling, it was using name instead of path * Add regression test * fixes #1024 * Bump core to 1.1.7.post3 — committed to polyaxon/polyaxon by mmourafiq 4 years ago
get_model_pathis wrong and should not be exposed the way is was exposed.By the way since you are logging the same artifact but with several versions, I would recommend the use of a name and the step arg, this will let you to create a single event and therefore a single lineage information with statistics, and you can always pull that event file as pipe separated values file or as a pd.Dataframe with information about the assets paths, timestamps and steps. See this example where a single event named
audioof typewavis logged at several steps, all assets related to that artifact are located under the same directory, and all information is organized in the event related to the assets:If you
log_artifactor any other event type with different names for every checkpoint, you will end up with different event instances. Sometimes that is the behavior that user is looking for, for example for a single file: you have a choice to copy it manually to the the context path if you don’t want Polyaxon to put it in the asset subdir.I will wait for the build and release a patch release
v1.1.7.post3This was a bug, this was the only function that had this issue:
nameinstead ofpathand there was no test for extracting the name from the path see this lineI fixed the issue and improved the extension and basename extraction to handle cases like
foo.tar.gz.Actually there’s tests I just did not check correctly.